8000 Fetchers docstring examples trigger dataset fetch in CI · Issue #28707 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Fetchers docstring examples trigger dataset fetch in CI #28707

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.

Sign up for GitHub

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

Jump to bottom
Closed
jeremiedbb opened this issue Mar 27, 2024 · 4 comments · Fixed by #28709
Closed

Fetchers docstring examples trigger dataset fetch in CI #28707

jeremiedbb opened this issue Mar 27, 2024 · 4 comments · Fixed by #28709
Assignees
@glemaitre
Labels
Build / CI Performance

Comments

@jeremiedbb
Copy link
Member

Docstring examples were recently added to the fetchers. This makes the doc tests executed by pytest actually fetch the datasets.
In the fetcher tests we took some precaution to not fetch the real datasets, see

scikit-learn/sklearn/datasets/tests/test_lfw.py

Line 40 in 1bbb228

def mock_data_home(tmp_path_factory):

It has a significant impact on the duration of the test suite (and probably on memory usage as well)

27.75s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_pairs
20.98s call     ::__init__.py::_lfw.py::sklearn.datasets._lfw.fetch_lfw_people
@jeremiedbb jeremiedbb mentioned this issue Mar 27, 2024
Merged
@lesteve
Copy link
Member
lesteve commented Mar 27, 2024

This is probably doable to skip doctests in sklearn/conftest.py pytest_collection_modifyitems based on SKLEARN_SKIP_NETWORK_TESTS.

@glemaitre
Copy link
Member

Uhm normally, I almost sure to have written some code to avoid this using some fixture.

@glemaitre
Copy link
Member

At list it was the intent behind this PR:

https://github.com/scikit-learn/scikit-learn/pull/28027/files#diff-3cdc9913d34276436e69cd506f11cdc77ef286069bd5465aaeeb73ac2bdee318

@glemaitre
Copy link
Member

And indeed, the fetch_lfw_pairs is not added as a fixture here:

scikit-learn/sklearn/conftest.py

Lines 71 to 80 in 1bbb228

dataset_fetchers = {
"fetch_20newsgroups_fxt": fetch_20newsgroups,
"fetch_20newsgroups_vectorized_fxt": fetch_20newsgroups_vectorized,
"fetch_california_housing_fxt": fetch_california_housing,
"fetch_covtype_fxt": fetch_covtype,
"fetch_kddcup99_fxt": fetch_kddcup99,
"fetch_olivetti_faces_fxt": fetch_olivetti_faces,
"fetch_rcv1_fxt": fetch_rcv1,
"fetch_species_distributions_fxt": fetch_species_distributions,
}

So this is a simple fix.

/take

@glemaitre glemaitre mentioned this issue Mar 27, 2024
Merged
@glemaitre glemaitre self-assigned this Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@glemaitre glemaitre

Labels
Build / CI Performance
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

MAINT avoid fetching lfw datasets when SKLEARN_SKIP_NETWORK_TESTS=1 glemaitre/scikit-learn
3 participants
@lesteve @glemaitre @jeremiedbb
0