Skip to content

MNT Moving _CurveScorer from model_selection to metrics#29216

Merged
jeremiedbb merged 4 commits intoscikit-learn:mainfrom
vitaliset:mnt_curve_scorer
Jul 25, 2024
Merged

MNT Moving _CurveScorer from model_selection to metrics#29216
jeremiedbb merged 4 commits intoscikit-learn:mainfrom
vitaliset:mnt_curve_scorer

Conversation

@vitaliset
Copy link
Copy Markdown
Contributor

@vitaliset vitaliset commented Jun 9, 2024

Towards #25639.

As we'll be leveraging the code of sklearn.model_selection._classification_threshold._CurveScorer into the implementation of the proposed sklearn.metrics.decision_threshold_curve, it makes sense to move the _CurveScorer to metrics as asked by @glemaitre in this comment.

This PR moves the code of _CurveScorer and _threshold_scores_to_class_labels and the tests to the new location.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 9, 2024

✔️ Linting Passed

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

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

return classes[map_thresholded_score_to_label[(y_score >= threshold).astype(int)]]


class _CurveScorer(_BaseScorer):
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.

I think that we should move those in the _score.py file. We already have the _Scorer there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I created this file because I thought that it made sense to have the new function to live here. But I'll move it.
Side question: do you think that sklearn.metrics.decision_threshold_curve should also be inside _score.py?

@vitaliset vitaliset requested a review from glemaitre July 3, 2024 10:57
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.

do you think that sklearn.metrics.decision_threshold_curve should also be inside _score.py

I'm not sure yet. I think that we are going to assess it. Since it will linked to plot, it will have its dedicated file.

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.

Copy link
Copy Markdown
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

Thanks @vitaliset

@jeremiedbb jeremiedbb merged commit 3b7734e into scikit-learn:main Jul 25, 2024
@vitaliset vitaliset deleted the mnt_curve_scorer branch July 25, 2024 13:32
MarcBresson pushed a commit to MarcBresson/scikit-learn that referenced this pull request Sep 2, 2024
…-learn#29216)

Co-authored-by: Guillaume Lemaitre <guillaume@probabl.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants