DOC Rework plot_roc.py example#24200
Conversation
cmarmo
left a comment
There was a problem hiding this comment.
Hi @ArturoAmorQ, thanks for your work.
I've made some comments, mainly about the format.
ogrisel
left a comment
There was a problem hiding this comment.
Here is a partial review. More to come in a few days hopefully.
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com>
glemaitre
left a comment
There was a problem hiding this comment.
I think that we can state that the example is much better :)
I really like the narrative, I find it intuitive.
I put some nitpicking that could clarify the code.
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
| # Interpolate all ROC curves at these points | ||
| mean_tpr = np.zeros_like(fpr_grid) | ||
|
|
||
| for i in range(n_classes): |
There was a problem hiding this comment.
If you don't want to use sp.interpolate.interp1d then we should add a comment that interp is doing linear interpolation. Another downside of using interp is that values need to be ordered.
There was a problem hiding this comment.
I am hesitating about what is the best, as we actually use np.interp to compute the tpr in the private function _binary_roc_auc_score. Also it would add boilerplate code below. Maybe the easiest is to indeed add a comment on linear interpolation.
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
ogrisel
left a comment
There was a problem hiding this comment.
Thanks @ArturoAmorQ for refactoring and polishing this example. It really looks good. Just a few more suggestions below:
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
|
What is missing so this could be merged? |
I still have to tweak the plot legends to reduce their overlapping with the figure, I just haven't had the time. But (hopefully) this will be ready to merge soon ;) thanks for your interest, @haiatn! |
|
Thanks @ArturoAmorQ LGTM. |
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Reference Issues/PRs
Fixes #13546.
Fixes #24288.
Takes #24192 into account.
What does this implement/fix? Explain your changes.
As discussed in #13546, the current state of the
roc_plot.pyexample gives different macro-averaged AUC thanroc_auc_scorebecause they use different averaging strategies. This PR aims to clarifying the different averaging strategies by implementing a "tutorialization".Any other comments?
Side effect: Implements notebook style as intended in #22406.