Skip to content

MAINT | API Clean up deprecations for 1.6: in make_scorer#30001

Merged
adrinjalali merged 3 commits into
scikit-learn:mainfrom
jeremiedbb:cln-deprecations-1.6-needs-proba-thresholds
Oct 14, 2024
Merged

MAINT | API Clean up deprecations for 1.6: in make_scorer#30001
adrinjalali merged 3 commits into
scikit-learn:mainfrom
jeremiedbb:cln-deprecations-1.6-needs-proba-thresholds

Conversation

@jeremiedbb
Copy link
Copy Markdown
Member

removed deprecated parameters needs_proba and needs_thresholds of make_scorer.
response_method=None was possible during the deprecation cycle to detect when both are set but is now useless since it's equivalent to predict. This PR therefore deprecates None and announce the change of default to "predict" in 1.8.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 3, 2024

✔️ Linting Passed

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

Generated for commit: 33b036c. Link to the linter CI: here

@glemaitre glemaitre self-requested a review October 8, 2024 17:15
Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

It looks good. Only a suggestion for the changelog and a question regarding the deprecation of None via the parameter validation.

Comment thread doc/whats_new/v1.6.rst Outdated

- |API| The default value of the `response_method` parameter of
:func:`metrics.make_scorer` will change from `None` to `"predict"` and `None` will be
removed in 1.8.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe we can also mention that in practice it will not change the behaviour of the function.

Comment thread sklearn/metrics/_scorer.py
Copy link
Copy Markdown
Member

@glemaitre glemaitre 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 @jeremiedbb

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants