DOC Improve descriptions of roc_curve-related dosctrings#31238
DOC Improve descriptions of roc_curve-related dosctrings#31238thomasjpfan merged 6 commits intoscikit-learn:mainfrom
Conversation
sklearn/metrics/_plot/roc_curve.py
Outdated
| stored as attributes. | ||
|
|
||
| Read more in the :ref:`User Guide <visualizations>`. | ||
| Read more in the :ref:`User Guide <roc_metrics>`. |
There was a problem hiding this comment.
For ROCCurveDisplay, the visualizations link does show how to use the visualization api with ROCCurveDisplay: https://scikit-learn.org/stable/visualizations.html#visualizations
This kind of depends on what a reader wants to learn. "What is ROC Curve" or "How to use RocCurveDisplay"
There was a problem hiding this comment.
We've resorted to linking both e.g.,
scikit-learn/sklearn/metrics/_plot/regression.py
Lines 25 to 28 in 7131d94
lucyleeow
left a comment
There was a problem hiding this comment.
This reads much better thanks!
Maybe we could consider linking both the visualization page and the metric page in the user guide?
sklearn/metrics/_ranking.py
Outdated
| @@ -1115,6 +1115,10 @@ def roc_curve( | |||
| fpr and tpr. `thresholds[0]` represents no instances being predicted | |||
There was a problem hiding this comment.
I still feel that the whole description of the thresholds attribute in the roc_curve can be improved to avoid redundancies.
I think your description is clearer than this one. Why don't we use your wording here (and maybe make it explicit it is threshold[0]), then in the versionchanged we can just say something like: "The arbitrary threshold at infinity is added."
|
@lucyleeow I slightly modified your suggestion in #31238 (comment), as the wording "For details" sounded as an understatement to me. I also decided to keep everything related to the threshold at infinity inside the scikit-learn/sklearn/metrics/_ranking.py Lines 339 to 341 in ce47cbc but also to avoid repeating ourselves. WDYT? |
lucyleeow
left a comment
There was a problem hiding this comment.
Thanks, this is much clearer! Some nits but LGTM!
sklearn/metrics/_ranking.py
Outdated
| Decreasing thresholds on the decision function used to compute | ||
| fpr and tpr. `thresholds[0]` represents no instances being predicted | ||
| and is arbitrarily set to `np.inf`. | ||
| fpr and tpr. |
There was a problem hiding this comment.
I think I prefer it being at least mentioned in the main description, but I am also okay with it the way it is now.
Co-authored-by: Lucy Liu <jliu176@gmail.com>
Reference Issues/PRs
Follow-up from #29151.
What does this implement/fix? Explain your changes.
As suggested in #29151 (comment), this PR:
drop_intermediateparameter to be more descriptive of what it actually does.versionchangeddirective to inform that the threshold at infinity was added in v1.3 (in FIX thresholds should not exceed 1.0 with probabilities inroc_curve#26194) This is also done for consistency with the description of thethresholdsattribute in thedet_curveintroduced in DOC Add missing directives to det_curve-related docstrings #31225.Any other comments?
I still feel that the whole description of the
thresholdsattribute in theroc_curvecan be improved to avoid redundancies.