Skip to content

Update metric-learn to work with the most recent scikit-learn when doing pytest #311

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

Closed
wdevazelhes opened this issue Mar 22, 2021 · 2 comments · Fixed by #313
Closed

Update metric-learn to work with the most recent scikit-learn when doing pytest #311

wdevazelhes opened this issue Mar 22, 2021 · 2 comments · Fixed by #313
Assignees
Labels

Comments

@wdevazelhes
Copy link
Member

wdevazelhes commented Mar 22, 2021

There seems to have been some changes in scikit-learn's new version (0.24): I have the following errors ModuleNotFoundError: No module named 'sklearn.utils.testing' when calling pytest on metric-learn (after a fresh clone of master on a conda env with scikit-learn 0.24). The reason is the one from this comment uber/causalml#285 (comment):

The sklearn.utils.testing module is deprecated in version 0.22 and will be removed in version 0.24. The corresponding classes / functions should instead be imported from sklearn.utils. Anything that cannot be imported from sklearn.utils is now part of the private API.

@wdevazelhes wdevazelhes self-assigned this Mar 22, 2021
@bellet
Copy link
Member

bellet commented Mar 22, 2021

Indeed! We can also still import private functions from sklearn.utils._testing

We should however ensure back-compatibility with previous versions of sklearn (since we allow scikit-learn>=0.20.3). One option is to do something similar to uber/causalml#283

@wdevazelhes
Copy link
Member Author

Indeed! We can also still import private functions from sklearn.utils._testing

We should however ensure back-compatibility with previous versions of sklearn (since we allow scikit-learn>=0.20.3). One option is to do something similar to uber/causalml#283

Thanks for the reference @bellet, I just opened a PR to try fix the imports using this technique, and add the travis job to ensure it is indeed still back-compatible #313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants