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

WIP: [python-package] use sklearn_compat for multi-version scikit-learn compatibilities #6740

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

glemaitre
Copy link
Contributor

This is a try to see if the tool provided in https://github.com/sklearn-compat/sklearn-compat could helped at maintenance.

For the moment this PR should be disregarded and do not need any review.

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre
fix

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre
@glemaitre glemaitre changed the title MAINT use sklearn_compat for multi-version scikit-learn compatibilities WIP use sklearn_compat for multi-version scikit-learn compatibilities Dec 8, 2024
@glemaitre glemaitre changed the title WIP use sklearn_compat for multi-version scikit-learn compatibilities WIP: [python-package] use sklearn_compat for multi-version scikit-learn compatibilities Dec 8, 2024

Verified

This commit was signed with the committer’s verified signature.
glemaitre Guillaume Lemaitre
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks.

Before spending more time on this, please do remember what I said on Bluesky (link)... I personally would only support something like this if it looked liked it'd significantly reduce the maintenance burden of supporting the range of Python and scikit-learn versions we are trying to support. You're welcome to use our CI for testing this, and I'll respect your request to not review it until you feel it's ready.

And please, run the following and commit it to this branch to save some CI resources for other development in the project (these jobs should not be relevant for this work):

git rm .appveyor.yml .github/workflows/r_package.yml .github/workflows/cuda.yml

@glemaitre
Copy link
Contributor Author

Before spending more time on this, please do remember what I said on Bluesky (link)... I personally would only support something like this if it looked liked it'd significantly reduce the maintenance burden of supporting the range of Python and scikit-learn versions we are trying to support. You're welcome to use our CI for testing this, and I'll respect your request to not review it until you feel it's ready.

@jameslamb I completely understand the trade-off and do not expect the PR to get through if it is not looking alright on your side. Up to now, I see that the main bottleneck was validate_data, the tags, and the tests. So at the end, you workaround could involve less changes that the file that we vendor.

I initially wanted to use the CI to assess how far we are with the compatibility file that we wrote so far.

And please, run the following and commit it to this branch to save some CI resources for other development in the project

Thanks for the pointers. I'll bypass those checks.

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

Successfully merging this pull request may close these issues.

None yet

2 participants