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

FEA SLEP006: Metadata routing for learning_curve #28975

Merged
merged 12 commits into from May 17, 2024

Conversation

StefanieSenger
Copy link
Contributor

This PR adds metadata routing to learning_curve by principally following the same pattern as in cross_validate.

Reference Issues/PRs

Towards #22893

Copy link

github-actions bot commented May 8, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: af8efad. Link to the linter CI: here

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

only nits, and merge conflicts, otherwise nice one!

sklearn/model_selection/_validation.py Show resolved Hide resolved
sample_weight="fit_sample_weight", metadata="fit_metadata"
)

if func is learning_curve:
Copy link
Member

Choose a reason for hiding this comment

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

I would move this to a separate test instead of adding to this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

sklearn/utils/_metadata_requests.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@StefanieSenger StefanieSenger left a comment

Choose a reason for hiding this comment

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

@adrinjalali Thanks for your review. I've made that little change.

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

@OmarManzoor @glemaitre could you have a look?

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved
Copy link
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @StefanieSenger

sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved
@glemaitre glemaitre changed the title FEA Metadata routing for learning_curve FEA SLEP006: Metadata routing for learning_curve May 16, 2024
Co-authored-by: Adrin Jalali <[email protected]>
Co-authored-by: Omar Salman <[email protected]>
@OmarManzoor OmarManzoor merged commit 77fc72c into scikit-learn:main May 17, 2024
30 checks passed
@StefanieSenger StefanieSenger deleted the metadata_learning_curve branch May 17, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants