Remove remote dataset retrieval for tests#946
Conversation
solegalli
left a comment
There was a problem hiding this comment.
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.
|
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 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! 😄 |
Remove all calls to
fetch_california_housingfrom 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.