8000 Use of private API will break with scikit-learn 0.22 · Issue #616 · scikit-learn-contrib/imbalanced-learn · GitHub
[go: up one dir, main page]
Skip to content

Use of private API will break with scikit-learn 0.22 #616

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
jnothman opened this issue Oct 24, 2019 · 8 comments · Fixed by #617
Closed

Use of private API will break with scikit-learn 0.22 #616

jnothman opened this issue Oct 24, 2019 · 8 comments · Fixed by #617
Labels
easy good first issue Indicates a good issue for first-time contributors Status: Help Wanted Indicates that a maintainer wants help on an issue or pull request

Comments

@jnothman
Copy link
Member

imblearn uses private scikit-learn API.

from sklearn.ensemble.base import _set_random_states

It will not be able to be imported in Scikit-learn version 0.22 due to sklearn.ensemble.base being renamed.

Ping @NicolasHug FYI
@jnothman
Copy link
Member Author

This is because the new ghost of ensemble.base (from scikit-learn/scikit-learn#14964) imports * from ensemble._base, which does not include _set_random_states

@chkoar
Copy link
Member
chkoar commented Oct 24, 2019

@jnothman Do you propose to make a utility function that will work across scikit-learn versions or vendorize that function?

@jnothman
Copy link
Member Author
jnothman commented Oct 24, 2019 via email

@NicolasHug
Copy link
NicolasHug commented Oct 24, 2019 •
edited
Loading

Thanks for the ping. I didn't realize but indeed, our renaming PRs will make it impossible to import any private module attribute since these are mangled.

There might be a way to make base.py still import the mangled names? Might not be worth it though: base.py will be removed in 0.24 so you might just as well update your code now to use _base.py. You'll have to do it eventually anyway.

@NicolasHug
Copy link

Also ping @adrinjalali @thomasjpfan @glemaitre I think you guys should know too ;)

@glemaitre
Copy link
Member

Basically, for the latest version, we always required the latest scikit-learn so it will not be an issue.
We might have some issue of old imbalanced-learn version with the latest scikit-learn, thought. In this case, we could raise an error asking to update the version.

We will need to make some maintenance for the next version (tools using keras and tensorflow are also brittle).

@adrinjalali
Copy link
Member

I suspect a few libraries would need to simply test import and in case of an error import from the old place. I don't think this is a major issue.

@jnothman
Copy link
Member Author

This should be fixed ASAP before sklearn 0.22 release....

@jnothman jnothman added easy good first issue Indicates a good issue for first-time contributors Status: Help Wanted Indicates that a maintainer wants help on an issue or pull request labels Oct 30, 2019
@glemaitre glemaitre mentioned this issue Oct 30, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
easy good first issue Indicates a good issue for first-time contributors Status: Help Wanted Indicates that a maintainer wants help on an issue or pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

MAINT compatibility with scikit-learn 0.22 glemaitre/imbalanced-learn
5 participants
@jnothman @NicolasHug @adrinjalali @glemaitre @chkoar
0