ENH add metadata routing to cross_val* #26896

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

glemaitre merged 29 commits into scikit-learn:main from adrinjalali:slep6/cross_val

Aug 9, 2023

Member

adrinjalali commented Jul 25, 2023 •

edited

Loading

This adds metadata routing to cross_val* functions.

Note that this PR does NOT route metadata for predict method of the estimator if any.

Closes #13432
Fixes #4632
Fixes #7646
Fixes #20349

adrinjalali added 5 commits

July 18, 2023 11:41


          ENH add metadata routing to cross_val*

c9fcd6e


          existing tests pass

9a0cfba


          handle groups and start tests


          Merge remote-tracking branch 'upstream/main' into slep6/cross_val

9d40ca8

...

0e64c4a

github-actions bot added module:model_selection module:utils labels

github-actions bot commented Jul 25, 2023 •

edited

Loading

✔️ Linting Passed

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

_{Generated for commit: 19c1193. Link to the linter CI: here}

adrinjalali mentioned this pull request

Unclear message regarding param validation #26897

Open

adrinjalali added 3 commits

July 25, 2023 23:04


          pass

8bf6ce5


          Merge remote-tracking branch 'upstream/main' into slep6/cross_val

185cc2d


          fix imports

f8528d1

adrinjalali mentioned this pull request

SLEP006 - Metadata Routing task list #22893

Open

28 tasks

adrinjalali added 8 commits

July 27, 2023 20:12

...

5d633d8


          fixes and tests

787b196


          Merge remote-tracking branch 'upstream/main' into slep6/cross_val

feacd04


          fix test to fit first

7bc37c7


          add cross_val_predict and tests

a62174f


          Merge remote-tracking branch 'upstream/main' into slep6/cross_val

f948747


          add changelog

813a63e


          minimal touch

d4cc661

adrinjalali marked this pull request as ready for review

August 1, 2023 13:09


          [doc build] fix internal deprecated usage

10c94b0

Member Author

adrinjalali commented Aug 1, 2023

@glemaitre @OmarManzoor @thomasjpfan this is ready for review.

glemaitre self-requested a review

August 2, 2023 14:13

glemaitre reviewed

View reviewed changes

sklearn/model_selection/tests/test_validation.py Show resolved Hide resolved

glemaitre reviewed

View reviewed changes

sklearn/model_selection/tests/test_validation.py Show resolved Hide resolved

glemaitre reviewed

View reviewed changes

sklearn/model_selection/tests/test_validation.py Show resolved Hide resolved

glemaitre reviewed

View reviewed changes

sklearn/model_selection/tests/test_validation.py Show resolved Hide resolved

glemaitre reviewed

View reviewed changes

sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved

glemaitre reviewed

View reviewed changes

Member

glemaitre left a comment

Just a couple of nitpicks. Otherwise, I am fine with the design.

In the future we could improve:

the code sharing between cross-valdiate and cross_val_predict
how _MultiMetricScorer is passed around. It would make sense to create such instance when checking and passing it around and to create a to_dict to convert a scorer back to what is needed later on.

adrinjalali and others added 8 commits

August 3, 2023 13:36


          Update sklearn/model_selection/tests/test_validation.py

ee03848

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>


          Update sklearn/model_selection/tests/test_validation.py

bf70ad0

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>


          Update sklearn/model_selection/tests/test_validation.py

66acee2

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>


          Update sklearn/model_selection/tests/test_validation.py

7ea0c58

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>


          Update sklearn/model_selection/tests/test_validation.py

b0bb9bb

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>


          comment on IDs

047510b


          Merge branch 'slep6/cross_val' of github.com:adrinjalali/scikit-learn…

77b0b02

… into slep6/cross_val


          Merge remote-tracking branch 'upstream/main' into slep6/cross_val

85e0a98

This was referenced Aug 3, 2023

make cross_val_predict use cross_validate #27000

Open

refactor cross_validate to pass around _MultimetricScorer #27001

Closed

Member Author

adrinjalali commented Aug 3, 2023

Created #27001 and #27000 to address the comments separately.

OmarManzoor reviewed

View reviewed changes

Contributor

OmarManzoor left a comment

Generally looks good. Added a few comments.

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved

adrinjalali added 2 commits

August 7, 2023 17:27


          apply Omar's suggestions

df62460


          Merge remote-tracking branch 'upstream/main' into slep6/cross_val

58b0ab1

OmarManzoor reviewed

View reviewed changes

Contributor

OmarManzoor left a comment

A few more comments.

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/_validation.py Outdated Show resolved Hide resolved

sklearn/model_selection/tests/test_validation.py Outdated Show resolved Hide resolved

adrinjalali added 2 commits

August 8, 2023 10:52


          more fixes by Omar

72a0537


          Merge remote-tracking branch 'upstream/main' into slep6/cross_val

19c1193

OmarManzoor approved these changes

View reviewed changes

Contributor

OmarManzoor left a comment

LGTM. Thanks @adrinjalali

glemaitre merged commit 6fa514a into scikit-learn:main

Member

glemaitre commented Aug 9, 2023

LGTM. Thanks @adrinjalali

adrinjalali deleted the slep6/cross_val branch

August 9, 2023 08:16

This was referenced Aug 9, 2023

Add sample_weights to setup pycaret/pycaret#1350

Open

Integration with sklearn tommyod/KDEpy#29

Open

Closes #480 dask/dask-ml#481

Open

Member

lorentzenchr commented Aug 9, 2023

I guess we can promote this to a major feature already.

Member Author

adrinjalali commented Aug 9, 2023

Soon I'll update the changelog to create a separate section for SLEP006 @lorentzenchr

TamaraAtanasoska pushed a commit to TamaraAtanasoska/scikit-learn that referenced this pull request


          ENH add metadata routing to cross_val* (scikit-learn#26896)

8c039ec

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

glemaitre mentioned this pull request

FEA metadata routing for permutation_test_score #27674

Closed

REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request


          ENH add metadata routing to cross_val* (scikit-learn#26896)

3cdb5c9

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

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

Labels

module:model_selection module:utils