Skip to content

Ensure all method signatures are sklearn compatible#6260

Merged
rapids-bot[bot] merged 3 commits intorapidsai:branch-25.02from
jcrist:fix-sklearn-signatures
Jan 27, 2025
Merged

Ensure all method signatures are sklearn compatible#6260
rapids-bot[bot] merged 3 commits intorapidsai:branch-25.02from
jcrist:fix-sklearn-signatures

Conversation

@jcrist
Copy link
Member

@jcrist jcrist commented Jan 24, 2025

sklearn requires fit/fit_transform/... always take a y parameter, even if it's ignored. This adds a test to ensure our signatures match this rule, and fixes any cases where they didn't. This makes it easier to include cuml estimators within sklearn pipelines.

Fixes #6255.

`sklearn` requires `fit`/`fit_transform`/... always take a `y`
parameter, even if it's ignored. This adds a test to ensure our
signatures match this rule, and fixes any cases where they didn't. This
makes it easier to include `cuml` estimators within sklearn pipelines.
@jcrist jcrist requested a review from a team as a code owner January 24, 2025 16:22
@jcrist jcrist requested review from bdice and teju85 January 24, 2025 16:22
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Jan 24, 2025
@jcrist jcrist added bug Something isn't working non-breaking Non-breaking change Cython / Python Cython or Python issue and removed Cython / Python Cython or Python issue labels Jan 24, 2025
Copy link
Contributor

@wphicks wphicks left a comment

Choose a reason for hiding this comment

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

Love anything that gets us closer to API compatibility! Nice change and solid implementation.

@jcrist
Copy link
Member Author

jcrist commented Jan 24, 2025

Failures look unrelated (and maybe transient). I lack permissions to restart the failed runs.

@bdice
Copy link
Contributor

bdice commented Jan 24, 2025

@jcrist You'll need to merge in the upstream to get #6199 to fix the devcontainer jobs (I did this for you to save time). Also, please request write permissions for yourself on this repo from the ops team so that you have privileges to rerun jobs.

@betatim
Copy link
Member

betatim commented Jan 27, 2025

Merged "main" into this PR to get the changes from #6227 which will solve the failed CI runs

@dantegd
Copy link
Member

dantegd commented Jan 27, 2025

/merge

@rapids-bot rapids-bot bot merged commit 3a32228 into rapidsai:branch-25.02 Jan 27, 2025
65 checks passed
@jcrist jcrist deleted the fix-sklearn-signatures branch May 13, 2025 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Cython / Python Cython or Python issue non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Not all estimators match scikit-learn signature conventions

5 participants