ENH Improve ROC curves visualization and add option to plot chance level #25972
Conversation
This is sometimes an annoying option on our side when we only want to push or merge a very small suggestion at the end of the PR. I would suggest always allowing maintainers to push into your PR, at least in the scikit-learn project. |
|
@glemaitre Sorry about that, this is a known issue of Github that I cannot give that permission if I'm not pushing from my own account. If that's inconvenient, I will close this one and start a new one. |
There was a problem hiding this comment.
For backward compatibility, we need to have plot_chance_level=False. Otherwise, we change the behaviour.
In addition of adding chance_level_kwargs, we need a new test to check that we alter the style of the chance_level_ line.
We also need to check the examples where RocCurveDisplay is used. We need to remove the part we were plotting the chance level by hand and instead use the new feature. You can have a look in the examples folder for the example that import the RocCurveDisplay.
| # Set limits of axes to [0, 1] and fix aspect ratio to squared | ||
| ax.set_xlim((0, 1)) | ||
| ax.set_ylim((0, 1)) | ||
| ax.set_aspect(1) |
There was a problem hiding this comment.
Let's move this change in another PR. We will need an additional entry in the changelog since we are fixing/improving the rendering.
There was a problem hiding this comment.
It should be shared with PR and ROC curve
| # Plot the frame in dotted line, so that the curve can be | ||
| # seen better when values are close to 0 or 1 | ||
| for s in ["right", "left", "top", "bottom"]: | ||
| ax.spines[s].set_linestyle((0, (1, 5))) | ||
| ax.spines[s].set_linewidth(0.5) |
There was a problem hiding this comment.
Same here. We can postpone the despining. It could also be shared between different type of plots. And we should be able to control it via some keywords.
|
|
||
| **kwargs : dict |
There was a problem hiding this comment.
Also add the documnetation for chance_level_kwargs
|
Okay, thanks for your review. I will make new PRs soon adopting these changes from my own account, so that you can make direct modifications. Again, sorry for the inconvenience I may have caused you. |
|
Please see #25987 |
Reference Issues/PRs
Fixes #25929.
What does this implement/fix? Explain your changes.
This implements the following visual improvements:
plot_chance_levelto indicate whether to plot the chance level. This option is available viaRocCurveDisplay.plot,RocCurveDisplay.from_estimator, andRocCurveDisplay.from_predictions.chance_level_to theRocCurveDisplay. If the chance level is plotted, the attribute would be the Matplotlib 2D line object of the chance level line. Otherwise, it would be None.Any other comments?
I may have misunderstood what @glemaitre asked me to do in Issue #25929. If I'm doing wrong, please tell me and I will close this issue and open new ones ASAP.
Also, I'm aware that reviewers may not have permission to directly modify this PR, because I'm making this PR from my course repo according to the course policy. However, I will make changes ASAP when the reviewer makes a comment. Sorry for the inconvenience I may have caused you.