-
-
Notifications
You must be signed in to change notification settings - Fork 26.5k
MNT Moving _CurveScorer from model_selection to metrics
#29216
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.
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
Conversation
| return classes[map_thresholded_score_to_label[(y_score >= threshold).astype(int)]] | ||
|
|
||
|
|
||
| class _CurveScorer(_BaseScorer): |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
glemaitre
left a comment
There was a problem hiding this 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.
glemaitre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
jeremiedbb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @vitaliset
…-learn#29216) Co-authored-by: Guillaume Lemaitre <[email protected]>
Towards #25639.
As we'll be leveraging the code of
sklearn.model_selection._classification_threshold._CurveScorerinto the implementation of the proposedsklearn.metrics.decision_threshold_curve, it makes sense to move the_CurveScorertometricsas asked by @glemaitre in this comment.This PR moves the code of
_CurveScorerand_threshold_scores_to_class_labelsand the tests to the new location.