FIX Avoid setting legend when labels are None in RocCurveDisplay kwargs

[ArturoAmorQ]

Reference Issues/PRs

What does this implement/fix? Explain your changes.

In #29617 and #29611 the RocCurveDisplay is used to visualize the roc curve across several cross-validation splits. As having the labels for each split is unnecessary, we rather set label=None, but that rises a UserWarning: No artists with labels found to put in legend due to the legend being set at a lower level inside the display.

This PR fixes the issue.

Any other comments?

Notice that "label" in chance_level_line_kw and "label" in line_kwargs are always True, as those keys are assigned earlier in the code. As "label" cannot be removed, only updated to None, we rather query for it's value using the get method.

[github-actions]

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: ca58f82. Link to the linter CI: here}

[glemaitre]

Isn't it an or and instead of an and? We need one of the two not being None to show the legend?

It also means that we don't have a test right now.

[ArturoAmorQ]

Yes, I did change the logic so that the legend is not set as soon as either of the labels is None.

The reasoning is that I don't think anyone would set plot_chance_lvl=True and override chance_level_kw with {"label": None}, so that part could be a bit more flexible. WDYT?

[glemaitre]

For me this is counter-intuitive to have no legend for the chance level curve with the following code:

In [3]: import matplotlib.pyplot as plt
   ...: from sklearn.datasets import make_classification
   ...: from sklearn.metrics import RocCurveDisplay
   ...: from sklearn.model_selection import train_test_split
   ...: from sklearn.svm import SVC
   ...: X, y = make_classification(random_state=0)
   ...: X_train, X_test, y_train, y_test = train_test_split(
   ...:     X, y, random_state=0)
   ...: clf = SVC(random_state=0).fit(X_train, y_train)
   ...: RocCurveDisplay.from_estimator(
   ...:    clf, X_test, y_test, label=None, plot_chance_level=True
   ...: )
   ...: plt.show()

If I really want to turn off the legend for the chance level then I can do it with the right params:

In [2]: import matplotlib.pyplot as plt
   ...: from sklearn.datasets import make_classification
   ...: from sklearn.metrics import RocCurveDisplay
   ...: from sklearn.model_selection import train_test_split
   ...: from sklearn.svm import SVC
   ...: X, y = make_classification(random_state=0)
   ...: X_train, X_test, y_train, y_test = train_test_split(
   ...:     X, y, random_state=0)
   ...: clf = SVC(random_state=0).fit(X_train, y_train)
   ...: RocCurveDisplay.from_estimator(
   ...:    clf, X_test, y_test, label=None, plot_chance_level=True, chance_level_kw={"label": None},
   ...: )
   ...: plt.show()

So to have this last behaviour you need to have a or operator and not a and operator.

[glemaitre]

We will need an additional test to check that the behaviour is the one that we want.

[ArturoAmorQ]

I removed this test as it is redundant with the assert chance_level_kw["label"] in legend_labels added further below.

[ArturoAmorQ]

I addressed all your comments @glemaitre. Also I don't really think we need a changelog entry here, unless you think otherwise.

[glemaitre]

I think that we need to have an entry in a changelog because we change the behaviour and this is actually a fix.

Apart from that. LGTM.

[glemaitre]

LGTM. Thanks @ArturoAmorQ

[OmarManzoor]

LGTM. Thanks @ArturoAmorQ