Skip to content

Remove remote dataset retrieval for tests#946

Draft
Nacho888 wants to merge 5 commits into
feature-engine:mainfrom
Nacho888:static-california-dataset-for-tests
Draft

Remove remote dataset retrieval for tests#946
Nacho888 wants to merge 5 commits into
feature-engine:mainfrom
Nacho888:static-california-dataset-for-tests

Conversation

@Nacho888

@Nacho888 Nacho888 commented Jul 18, 2026

Copy link
Copy Markdown

Remove all calls to fetch_california_housing from the tests, so that no internet connection is required for the execution of these. This is a small optimization but more in line with the best practices of avoiding making these type of connections while executing the test suite.

It removes an external dependency, and might make it slightly faster - at the expense of the addition of an extra file. This file will only be available for tests and the won't be bundled in the end-user package.

This pattern might need to be extended to other tests that make use of other remote datasets.

@Nacho888
Nacho888 marked this pull request as ready for review July 18, 2026 14:43

@solegalli solegalli left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @Nacho888

Thank you very much for this PR. We definitely need to step away from making urlib calls in our tests.

I think for simpler transformers like the discretiser, we could replace the california housing by a hard coded dataframe, and then we don't store a csv file.

For more complex transformers, like MRMR, we could try creating an artificial dataset with make_classification and see if that resolves the issue, before committing to storing a csv file as part of the test suite.

It would be good as a start, to understand which tests rely of this dataset, so we know how big a change we need if we were to replace it completely.

@Nacho888

Nacho888 commented Jul 25, 2026

Copy link
Copy Markdown
Author

Hi @solegalli, sorry for the late response!

I completely agree with you that moving away from the CSV and instead using a static dataframe in a fixture is a cleaner approach that removes the need to keep this extra file as part of the repository. Also, for the more complex transformers, it would be good to experiment with the make_classification function as you say.

Maybe I should've opened this PR in draft (it was a little bit rushed during the EuroPython sprints!), as it is better to first identify which tests depend on remote dependencies. Plainly, these should be detected by just running the suite without an internet connection - then those can be listed, and the corresponding action can be taken depending on the complexity of each test.

I will try to get some time in the upcoming days to at least do the analysis mentioned above. For now, I am moving this into draft, and when a proper plan is decided, I will refine an issue to link to this PR that has all necessary information; then we can proceed here with resolution and re-publish it.

Thanks, I will be in touch! 😄

@Nacho888
Nacho888 marked this pull request as draft July 25, 2026 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants