DOC Improve descriptions of roc_curve-related dosctrings #31238
Conversation
sklearn/metrics/_plot/roc_curve.py
Outdated
| @@ -20,7 +20,7 @@ class RocCurveDisplay(_BinaryClassifierCurveDisplayMixin): | |||
| a :class:`~sklearn.metrics.RocCurveDisplay`. All parameters are | |||
| 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
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? |
sklearn/metrics/_ranking.py
Outdated
| @@ -1112,8 +1112,12 @@ def roc_curve( | |||
|
|
|||
| thresholds : ndarray of shape (n_thresholds,) | |||
| 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.