-
-
Notifications
You must be signed in to change notification settings - Fork 26k
ENH follow-up style improvements of DET curves #10591 #18169
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
ping @cmarmo @agramfort @jnothman who reviewed #10591 |
We should open an issue to add the |
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.
nits but lgtm, thanks @glemaitre
@@ -1,7 +1,7 @@ | |||
""" | |||
======================================= | |||
==================================== |
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.
should this be in metric
instead of model_selection
?
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.
The plot_roc
is also in this section. I am not sure what is best. I think that the idea is to compare model and select the best one in some sense. So it is not so wrong to put it in this section.
There's also the "False Negative Rate" ylabel that overlaps with the left plot in the example. |
ping @lorentzenchr @rth as well :P |
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.
forgot to approve
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.
forgot to approve
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.
Don't know how to add a review comment to the following:
- In the docstring of
detection_error_tradeoff_curve
, replace "Note: This metrics is used for ranking evaluation of a binary" by "This metric is used for evaluation of ranking and error tradeoffs of a binary". - In the User Guide, can we replace the word "ordinate" by "y-axis"?
@lorentzenchr Done |
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.
nitpick, otherwise LGTM
Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>
Good to be merged :) |
…ikit-learn#18169) * follow-up on scikit-learn#10591 * DOC improvements * TST add additional test for pos_label Co-authored-by: Christian Lorentzen <lorentzen.ch@gmail.com>
Follow-up of #10591.
Closes #18174.