8000 FEA SLEP006: Metadata routing for `learning_curve` by StefanieSenger · Pull Request #28975 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

FEA SLEP006: Metadata routing for learning_curve #28975

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
Merged
merged 12 commits into from
May 17, 2024
Merged

FEA SLEP006: Metadata routing for learning_curve #28975

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

@github-actions GitHub Actions
Copy link
github-actions bot commented May 8, 2024 •
edited
Loading

✔️ Linting Passed

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

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

adrinjalali
adrinjalali reviewed May 14, 2024
View reviewed changes
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/tests/test_validation.py Outdated
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.

StefanieSenger
StefanieSenger commented May 15, 2024
View reviewed changes
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.

adrinjalali
adrinjalali approved these changes May 15, 2024
View reviewed changes
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?

OmarManzoor
OmarManzoor approved these changes May 15, 2024
View reviewed changes
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

@glemaitre glemaitre changed the title FEA Metadata routing for learning_curve FEA SLEP006: Metadata routing for learning_curve May 16, 2024
@glemaitre glemaitre mentioned this pull request May 16, 2024
Open
28 tasks
@StefanieSenger @adrinjalali @OmarManzoor
Apply suggestions from code review
af8efad 
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Omar Salman <omar.salman@arbisoft.com>
@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
@jeremiedbb jeremiedbb mentioned this pull request May 20, 2024
Merged
14 tasks
@StefanieSenger StefanieSenger mentioned this pull request Jun 21, 2024
Merged
@jeremiedbb jeremiedbb mentioned this pull request Jul 2, 2024
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali adrinjalali approved these changes

@OmarManzoor OmarManzoor OmarManzoor approved these changes

Assignees
No one assigned
Labels
module:model_selection module:utils
Projects
Metadata routing
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@StefanieSenger @adrinjalali @OmarManzoor
0