Skip to content
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

Upgrade to comply with scikitlearn >1.3 #44

Merged
merged 5 commits into from
Dec 23, 2024

Conversation

Richie94
Copy link
Contributor

@Richie94 Richie94 commented Dec 4, 2024

With scikit learn 1.3 if_delegate_has_method has been removed in favor if if_available.
See scikit-learn/scikit-learn#20506

Therefore i updated the scikit learn version to above 1.3, and changed the calls to the ones i think should be used now.

@Richie94
Copy link
Contributor Author

Richie94 commented Dec 4, 2024

Should fix #42

@solegalli
Copy link
Contributor

Hey @Richie94

Thanks for the PR with the changes!

Could you please remove the modified reqs file from this PR? We are updating the library as we speak and we are managing the new dependencies elsewhere. See #45

@lopuhin this PR will probably not pass the tests, because we have dependencies on older versions of sklearn where we use the boston dataset. I suggest we keep it on hold while we fix the tests.

@Richie94
Copy link
Contributor Author

Richie94 commented Dec 5, 2024

it has been introduces in sklearn 1.0.0, so it wont run with old versions... im not mainly a python def, so im not sure on how you want to achieve compatibility with old versions.
0.20 is from 2019, its nearly 6 years ago, i think raising the requirements in a new release can be considered...
but ill remove that, so you can handle it like you want to...

@solegalli
Copy link
Contributor

Thank you @Richie94

No problem at all, the next release of eli5 will only be compatible with sklearn 1.5 and above.

@lopuhin this is good to merge.

I said to put it on hold, but actually we need these changes to be able to runs the tests to begin with.

@solegalli
Copy link
Contributor

@lopuhin @kmike please merge :)

@lopuhin
Copy link
Contributor

lopuhin commented Dec 23, 2024

Thanks @Richie94 and @solegalli , and sorry @solegalli I missed the message, appreciate a reminder

@lopuhin lopuhin merged commit 8eed51a into eli5-org:master Dec 23, 2024
0 of 10 checks passed
@solegalli
Copy link
Contributor

awesome thank you!

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.

3 participants