-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
API add from_estimator and from_preditions to PrecisionRecallDisplay #20552
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
API add from_estimator and from_preditions to PrecisionRecallDisplay #20552
Conversation
@@ -0,0 +1,138 @@ | |||
import pytest |
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.
Note to reviewer: this test will be used as well for the RocCurveDisplay
and the DetCurveDisplay
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.
Thank you for working on this @glemaitre !
Few comments, otherwise LGTM.
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! Could you please resolve merge conflicts? I'll review tests when that is done..
Co-authored-by: Roman Yurchak <rth.yurchak@gmail.com>
…Display' into class_methods_PrecisionRecallDisplay
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.
Generally LGTM. Not sure why codecov doesn't show in CI, but the report is in https://app.codecov.io/gh/scikit-learn/scikit-learn/compare/20552
@@ -144,7 +152,206 @@ def plot(self, ax=None, *, name=None, **kwargs): | |||
self.figure_ = ax.figure | |||
return self |
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 still find it counter-intuitive that plot returns self instead of ax, but that's unrelated to this PR
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 don't really recall if there is any reason. Maybe it allows pipelining some operations but it is true that pandas/seaborn usually return an axis
There is 2 approvals here to I will merge this one to fix the conflict in #20569 |
…cikit-learn#20552) Co-authored-by: Roman Yurchak <rth.yurchak@gmail.com>
Address some of #15880
Add the class methods
from_estimator
andfrom_predictions
to the classPrecisionRecallDisplay
. In addition, we deprecate theplot_precision_recall_curve
in favour of these two class methods.TODO: