Skip to content

[API, MAINT] Deprecate usage of y_prob and probas_pred in sklearn.metrics#28092

Merged
thomasjpfan merged 29 commits intoscikit-learn:mainfrom
adam2392:ndim
Feb 16, 2024
Merged

[API, MAINT] Deprecate usage of y_prob and probas_pred in sklearn.metrics#28092
thomasjpfan merged 29 commits intoscikit-learn:mainfrom
adam2392:ndim

Conversation

@adam2392
Copy link
Copy Markdown
Member

Reference Issues/PRs

Fixes: #27994

What does this implement/fix? Explain your changes.

Consolidates usage of y_score or y_proba.

Any other comments?

Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 10, 2024

✔️ Linting Passed

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

Generated for commit: 62dc74e. Link to the linter CI: here

Copy link
Copy Markdown
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.

All versions need to be bumped by 1

Otherwise LGTM.

Comment on lines +3137 to +3140
"y_proba": ["array-like", None],
"sample_weight": ["array-like", None],
"pos_label": [Real, str, "boolean", None],
"y_prob": ["array-like", StrOptions({"deprecated"})],
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.

None and "deprecated" both need to be a hidden option.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

Comment on lines +868 to +874
"y_score": ["array-like", None],
"pos_label": [Real, str, "boolean", None],
"sample_weight": ["array-like", None],
"drop_intermediate": ["boolean"],
"probas_pred": [
"array-like",
StrOptions({"deprecated"}),
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.

same here, hidden options.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

adam2392 and others added 6 commits January 25, 2024 09:03
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Copy link
Copy Markdown
Member Author

@adam2392 adam2392 left a comment

Choose a reason for hiding this comment

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

Addressed your comments. Thanks @adrinjalali !

Comment on lines +3137 to +3140
"y_proba": ["array-like", None],
"sample_weight": ["array-like", None],
"pos_label": [Real, str, "boolean", None],
"y_prob": ["array-like", StrOptions({"deprecated"})],
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

Comment on lines +868 to +874
"y_score": ["array-like", None],
"pos_label": [Real, str, "boolean", None],
"sample_weight": ["array-like", None],
"drop_intermediate": ["boolean"],
"probas_pred": [
"array-like",
StrOptions({"deprecated"}),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@adam2392 adam2392 requested a review from adrinjalali January 25, 2024 18:39
Copy link
Copy Markdown
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.

LGTM. Thanks @adam2392

@adrinjalali
Copy link
Copy Markdown
Member

@glemaitre this should be a quick review.

Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @adam2392 !

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
@adam2392
Copy link
Copy Markdown
Member Author

Thank you for the review!

Signed-off-by: Adam Li <adam2392@gmail.com>
@thomasjpfan
Copy link
Copy Markdown
Member

The CI failures look related.

Signed-off-by: Adam Li <adam2392@gmail.com>
@adam2392
Copy link
Copy Markdown
Member Author

The CI failures look related.

Ah yes. The check needed to handle various edge cases properly since the variable can be an array, or a string. It should be fixed hopefully now...

Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
@adam2392 adam2392 requested a review from thomasjpfan February 16, 2024 17:40
@thomasjpfan thomasjpfan merged commit 0b0bddd into scikit-learn:main Feb 16, 2024
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.

Consolidation of the naming of y_pred_proba, y_score vs probas_pred

3 participants