Skip to content

DOC Add info when scoring = None in cross_validate #30303

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 2 commits into from
Nov 20, 2024
Merged

DOC Add info when scoring = None in cross_validate #30303

merged 2 commits into from
Nov 20, 2024

Conversation

lucyleeow
Copy link
Member

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Adds info for when when scoring = None in cross_validate to docstring.

Any other comments?

@lucyleeow lucyleeow added the Quick Review For PRs that are quick to review label Nov 19, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 19, 2024 •
edited
Loading

✔️ Linting Passed

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

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

@StefanieSenger
Copy link
Contributor

StefanieSenger commented Nov 19, 2024 •
edited
Loading

Oh wonderful! ❤️

I can't believe this wasn't there before.

And upon lookup, there are many other places, where this information is lacking: RFECV, SequentialFeatureSelector, LogisticRegressionCV, RidgeClassifierCV, LearningCurveDisplay, ValidationCurveDisplay, GridSearchCV, learning_curve, validation_curve.
And RidgeCV has a confusing wording for scoring=None.

virchan
virchan reviewed Nov 19, 2024
View reviewed changes
sklearn/model_selection/_validation.py Outdated
@@ -169,7 +169,7 @@ def cross_validate(

scoring : str, callable, list, tuple, or dict, default=None
Strategy to evaluate the performance of the cross-validated model on
the test set.
the test set. If None, the `estimator` object's `score` method is used.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
the test set. If None, the `estimator` object's `score` method is used.
the test set. If `None`, the `estimator`'s `.score()` method will be used.

WDYT?

ArturoAmorQ
ArturoAmorQ reviewed Nov 19, 2024
View reviewed changes
sklearn/model_selection/_validation.py Outdated
@@ -169,7 +169,7 @@ def cross_validate(

scoring : str, callable, list, tuple, or dict, default=None
Strategy to evaluate the performance of the cross-validated model on
the test set.
the test set. If None, the `estimator` object's `score` method is used.
Copy link
Member

Choose a reason for hiding this comment

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

Good catch! I would word it something more inline with the definition of the Estimator score method and maybe even link to that page of the user guide. WDYT of something similar to:

If None, the default evaluation criterion of the estimator is used.

Copy link
Member Author

@lucyleeow lucyleeow Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yup, that's a good idea, it gives more info/context as the user does not necessarily know what 'score' method implies. Changed (and link added).

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think @ArturoAmorQ's recommendation is better!

ArturoAmorQ
ArturoAmorQ approved these changes Nov 20, 2024
View reviewed changes
Copy link
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Thanks @lucyleeow, merging!

@ArturoAmorQ ArturoAmorQ merged commit d2e7220 into scikit-learn:main Nov 20, 2024
30 checks passed
@lucyleeow lucyleeow deleted the doc_cross_val branch November 20, 2024 10:20
@lucyleeow
Copy link
Member Author

Thank you! I'm going to take a look at the others @StefanieSenger mentioned and make sure the documentation of this particular item is consistent!

This was referenced Nov 21, 2024
Merged
Merged
jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Dec 4, 2024
@lucyleeow @jeremiedbb
DOC Add info when scoring = None in cross_validate (scikit-learn#…
fdcd3c2 
…30303)
jeremiedbb pushed a commit that referenced this pull request Dec 6, 2024
@lucyleeow @jeremiedbb
DOC Add info when scoring = None in cross_validate (#30303)
0e68084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@virchan virchan virchan left review comments

@ArturoAmorQ ArturoAmorQ ArturoAmorQ approved these changes

Assignees
No one assigned
Labels
Documentation module:model_selection Quick Review For PRs that are quick to review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lucyleeow @StefanieSenger @virchan @ArturoAmorQ