8000 FIX remove pytest warning in dataset/tests/test_common.py by glemaitre · Pull Request #17828 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

FIX remove pytest warning in dataset/tests/test_common.py 8000 #17828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 12, 2020

Conversation

glemaitre
Copy link
Member

Remove the PytestWarning in test_common.py from dataset

@glemaitre glemaitre mentioned this pull request Jul 3, 2020
< 8000 div class="d-inline-flex flex-row flex-items-center text-small"> 4 tasks
@@ -94,7 +94,8 @@ def _generate_func_supporting_param(param, dataset_type=("load", "fetch")):
condition=name.startswith("fetch") and _has_network(),
reason="Skip because fetcher requires internet network",
)]
marks.append(markers_fetch.get(name, pytest.mark.basic))
if name in markers_fetch:
marks.append(markers_fetch[name])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glemaitre maybe I'm misunderstanding the link you provided, but shouldn't the pytest.mark.network be added here? The network marker is defined in conftest.py.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The network is the line just above. But we are going to refactor this part with #17553

Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thomasjpfan thomasjpfan merged commit 36c3c2b into scikit-learn:master Jul 12, 2020
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0