Repository: scikit-learn/scikit-learn
Pull request: ENH/DEPR add new response_method and deprecate needs_* in make_scorer
closes #26817

While working in https://github.com/scikit-learn/scikit-learn/pull/26120, I find myself doing dirty tricks to overwrite the response method used in `_ThresholdScorer` while we should be able to pass a list of priorities and delegate to `_get_response_values`.

I introduce `response_method` in `make_scorer` that remove the need to `needs_proba` and `needs_threshold`. I assume that the API is simplified due to the work done with `_get_response_values`. Now, we don't need to have different type of scorers (i.e. `_PredictScorer`, `_ProbaScorer`, and `ThresholdScorer`). All are handled in a single class because all the complexity is delegated to the `_get_response_values` to which we provide the response method to try.

I need to alternate the provided `scorer` in the `_MultiMetricScorer` to be sure to hit the cache. It means that we need to decide on a single method to use for a single estimator. At the end, it simplifies also when the cache policy is beneficial.


Problem statement:
Scorer not working on ClassifierChain
### Describe the bug

ClassifeirChain fails when trying to make scoring predictions. The reason is: ClassifeirChain's attribute `classes_` is a list and not a np.array as the `_get_response_values` function expects. 

### Steps/Code to Reproduce

```python
from sklearn.multioutput import ClassifierChain
from sklearn.metrics._scorer import get_scorer
from sklearn.datasets import make_multilabel_classification
from sklearn.linear_model import LogisticRegression
from sklearn.model_selection import train_test_split
from sklearn.multioutput import ClassifierChain


X, Y = make_multilabel_classification(n_samples=72, n_classes=3, random_state=0)
X_train, X_test, Y_train, Y_test = train_test_split(X, Y, random_state=0)

base_lr = LogisticRegression(solver='lbfgs', random_state=0)
chain = ClassifierChain(base_lr, order='random', random_state=0)
y_pred = chain.fit(X_train, Y_train).predict(X_test)

print(get_scorer("average_precision")(chain, X_test, Y_test))
```

### Expected Results

No error thrown.

### Actual Results

```
  File "C:\Users\Mavs\Documents\Python\ATOM\test.py", line 204, in <module>
    print(get_scorer("average_precision")(chain, Y_test, y_pred))
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\metrics\_scorer.py", line 266, in __call__
    return self._score(partial(_cached_call, None), estimator, X, y_true, **_kwargs)
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\metrics\_scorer.py", line 466, in _score
    y_pred = method_caller(clf, "predict_proba", X, pos_label=pos_label)
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\metrics\_scorer.py", line 86, in _cached_call
    result, _ = _get_response_values(
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\utils\_response.py", line 77, in _get_response_values
    if pos_label is not None and pos_label not in classes.tolist():
AttributeError: 'list' object has no attribute 'tolist'
```
### Versions

```shell
System:
    python: 3.10.2 (tags/v3.10.2:a58ebcc, Jan 17 2022, 14:12:15) [MSC v.1929 64 bit (AMD64)]
executable: C:\Users\Mavs\Documents\Python\ATOM\venv310\Scripts\python.exe
   machine: Windows-10-10.0.19045-SP0
Python dependencies:
      sklearn: 1.3.0
          pip: 23.1.2
   setuptools: 59.8.0
        numpy: 1.23.5
        scipy: 1.10.1
       Cython: 0.29.30
       pandas: 1.5.3
   matplotlib: 3.7.2
       joblib: 1.3.1
threadpoolctl: 3.1.0
Built with OpenMP: True
threadpoolctl info:
       user_api: blas
   internal_api: openblas
         prefix: libopenblas
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\numpy\.libs\libopenblas.FB5AE2TYXYH2IJRDKGDGQ3XBKLKTF43H.gfortran-win_amd64.dll
        version: 0.3.20
threading_layer: pthreads
   architecture: Zen
    num_threads: 16
       user_api: openmp
   internal_api: openmp
         prefix: vcomp
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\sklearn\.libs\vcomp140.dll
        version: None
    num_threads: 16
       user_api: blas
   internal_api: openblas
         prefix: libopenblas
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\scipy.libs\libopenblas-802f9ed1179cb9c9b03d67ff79f48187.dll
        version: 0.3.18
threading_layer: pthreads
   architecture: Zen
    num_threads: 16
       user_api: openmp
   internal_api: openmp
         prefix: libiomp
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\torch\lib\libiomp5md.dll
        version: None
    num_threads: 8
       user_api: openmp
   internal_api: openmp
         prefix: libiomp
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\torch\lib\libiompstubs5md.dll
        version: None
    num_threads: 1
```

Hints from issue discussion:
Thanks for reporting this. I can reproduce the problem.

A slight complication is that `ClassifierChain.classes_` is defined as a list of the classes for each estimator in the chain. This is a bit different from what `classes_` should be IMHO. I think it makes sense to fix this and stick to the uniform interface for `classes_`. This might need some deprecation notice/warning. Maybe we need a new attribute that stores the individual classes seen by each estimator?

Would you be interested in submitting a Pull Request for this?
Hi @betatim ,

I would love to work on the proposed solution.
I am willing to give it a try as well, although I'll probably need some guidance about the deprecation/warning part.

I also noticed that `MultiOutputClassifier` experiences the same issue, and the docstring for the `classes_` attribute is wrong. It says `classes_: ndarray of shape (n_classes,)` but the code is the same as ClassifierChain.

https://github.com/scikit-learn/scikit-learn/blob/7f9bad99d6e0a3e8ddf92a7e5561245224dab102/sklearn/multioutput.py#L539

Is it possible for the base estimators to have different `classes_`? If not, it would make sense to let the attribute of the meta-estimator return the classes from the first base estimator.
> Is it possible for the base estimators to have different `classes_`? If not, it would make sense to let the attribute of the meta-estimator return the classes from the first base estimator.

It seems unlikely that they'd have different classes, but not impossible? I think the same data is passed to all estimators in the chain, but maybe if the estimator in the chain does some splitting of the data at random during its learning it would see different classes? I'm not sure if this is realistic or not. I think I'd go for the chain's `classes_` attribute being the union of all the individual estimator's `classes_`?

I'm not very familiar with the `ClassifierChain` methodology, so for me this would be a case where doing the research takes more time than making the change.

@tvdboom and @sqali regarding making a PR: maybe one of you can make a PR and the other can help review it?

ps. it might be worth making one PR for `ClassifierChain` and in it mentioning that `MultiOutputClassifier` has the same problem, but holding off with changing its code. Once there is consensus around the solution for `ClassifierChain` you can then add on the `MultiOutputClassifier` or make a second PR to deal with it.
Hi @betatim ,

Sure, I and @tvdboom would love to work on it together.

I will do my research and update it here.
So there are some puzzling stuff here.

First, it seems that the `ClassifierChain` accepts multiclass-multioutput while it should not. This is not the purpose of the original paper. The idea is to have a set of binary classifiers that can learn a label correlation. So I would think that we should run a `type_of_target` and reject if we are not in the `multilabel-indicator` case.

Secondly, for the classes, we expect each binary classifier to be trained on `[0, 1]`. The current version of the `classes_` is actually not informative at all. Making the union would already be more aligned with our own API but I don't think this is informative as well.

As a user, I would expect `classes_` to refer to the label's name which in the simple case would be `range(Y.shape[1])` (that corresponds as well to `np.unique(LabelBinarizer().inverse_transform(Y))`. In a world that `Y`, could be a dataframe, I would therefore expect `Y.columns`.

And I lost track of the original problem: `_get_response_values` is not written to handle multilabel classification. We can decide if we want to change the API of `classes_` for the multilabel but otherwise we should fix:

https://github.com/scikit-learn/scikit-learn/blob/fa3ca7ab9f4e1363c9f67a12652c362d53afb9cb/sklearn/utils/_response.py#L73-L75

If `estimator.classes_` is actually a list, then we can fork in a branch to handle this case. There is no problem with `pos_label` but I would need to check if there are any metrics supporting the output of `decision_function` or `predict_proba` in this case. `predict` should work like a charm just by returning directly the output.
Feel free to make the pr @sqali and add me to the reviewers
Hi @glemaitre @betatim ,

I can confirm that the estimator.classes_ is actually a list. I ran the below code and got the output.

```
X, Y = make_multilabel_classification(n_samples=72, n_classes=3, random_state=0)
X_train, X_test, Y_train, Y_test = train_test_split(X, Y, random_state=0)

base_lr = LogisticRegression(solver='lbfgs', random_state=0)
chain = ClassifierChain(base_lr, order='random', random_state=0)
y_pred = chain.fit(X_train, Y_train).predict(X_test)

print(chain.classes_)
print(type(chain.classes_))
```

Ouput:-
```
[array([0, 1]), array([0, 1]), array([0, 1])]
<class 'list'>
```

I am new to open source. I require some guidance here. I understand that I need to make changes in the multioutput.py file in the fit function of classes **MultiOutputClassifier** and **ClassifierChain** by introducing a new attribute to store the individual classes seen by each estimator. Kindly confirm if this would be the right approach.

If you are new to open-source @sqali it might be better to start with a [good first issue](https://github.com/scikit-learn/scikit-learn/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3A%22good+first+issue%22). Those are meant for newcomers compared to issues like this.

Commit message: TST add mulitlabel test

Patch to review:
diff --git a/doc/whats_new/v1.4.rst b/doc/whats_new/v1.4.rst
index c2b7d19404af9..478fc8a1f1c2a 100644
--- a/doc/whats_new/v1.4.rst
+++ b/doc/whats_new/v1.4.rst
@@ -111,6 +111,17 @@ Changelog
   to :ref:`metadata routing user guide <metadata_routing>`. :pr:`26789` by
   `Adrin Jalali`_.
 
+:mod:`sklearn.metrics`
+......................
+
+- |Enhancement| add a parameter `response_method` to define a list of priority
+  of response methods to use with metrics requiring the option `needs_threshold=True`.
+  :pr:`26840` by :user:`Guillaume Lemaitre <glemaitre>`.
+
+- |Fix| scorers used with :func:`metrics.get_scorer` handles properly
+  multilabel-indicator matrix.
+  :pr:`26840` by :user:`Guillaume Lemaitre <glemaitre>`.
+
 :mod:`sklearn.model_selection`
 ..............................
 
diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py
index c8969acba1744..0aae306d33365 100644
--- a/sklearn/metrics/_scorer.py
+++ b/sklearn/metrics/_scorer.py
@@ -25,8 +25,6 @@
 from inspect import signature
 from traceback import format_exc
 
-import numpy as np
-
 from ..base import is_regressor
 from ..utils import Bunch
 from ..utils._param_validation import HasMethods, StrOptions, validate_params
@@ -40,6 +38,7 @@
     process_routing,
 )
 from ..utils.multiclass import type_of_target
+from ..utils.validation import _check_response_method
 from . import (
     accuracy_score,
     average_precision_score,
@@ -130,7 +129,21 @@ def __call__(self, estimator, *args, **kwargs):
                 **{name: Bunch(score=kwargs) for name in self._scorers}
             )
 
-        for name, scorer in self._scorers.items():
+        # to have the highest cache hit rate, we need to check if we have any
+        # _ThresholdScorer and choose a single response method based on the current
+        # estimator
+        scorers = copy.deepcopy(self._scorers)
+        for name, scorer in scorers.items():
+            if isinstance(scorer, _ThresholdScorer):
+                if scorer._response_method is None:
+                    response_method = ("decision_function", "predict_proba")
+                else:
+                    response_method = scorer._response_method
+                scorer._response_method = _check_response_method(
+                    estimator, response_method
+                ).__name__
+
+        for name, scorer in scorers.items():
             try:
                 if isinstance(scorer, _BaseScorer):
                     score = scorer._score(
@@ -405,6 +418,10 @@ def _factory_args(self):
 
 
 class _ThresholdScorer(_BaseScorer):
+    def __init__(self, score_func, sign, kwargs, response_method=None):
+        super().__init__(score_func, sign, kwargs)
+        self._response_method = response_method
+
     def _score(self, method_caller, clf, X, y, **kwargs):
         """Evaluate decision function output for X relative to y_true.
 
@@ -452,20 +469,15 @@ def _score(self, method_caller, clf, X, y, **kwargs):
             raise ValueError("{0} format is not supported".format(y_type))
 
         if is_regressor(clf):
-            y_pred = method_caller(clf, "predict", X)
+            response_method = self._response_method or "predict"
+            y_pred = method_caller(clf, response_method, X)
         else:
+            if self._response_method is None:
+                response_method = ("decision_function", "predict_proba")
+            else:
+                response_method = self._response_method
             pos_label = self._get_pos_label()
-            try:
-                y_pred = method_caller(clf, "decision_function", X, pos_label=pos_label)
-
-                if isinstance(y_pred, list):
-                    # For multi-output multi-class estimator
-                    y_pred = np.vstack([p for p in y_pred]).T
-
-            except (NotImplementedError, AttributeError):
-                y_pred = method_caller(clf, "predict_proba", X, pos_label=pos_label)
-                if isinstance(y_pred, list):
-                    y_pred = np.vstack([p[:, -1] for p in y_pred]).T
+            y_pred = method_caller(clf, response_method, X, pos_label=pos_label)
 
         scoring_kwargs = {**self._kwargs, **kwargs}
         return self._sign * self._score_func(y, y_pred, **scoring_kwargs)
@@ -638,6 +650,11 @@ def _check_multimetric_scoring(estimator, scoring):
         "greater_is_better": ["boolean"],
         "needs_proba": ["boolean"],
         "needs_threshold": ["boolean"],
+        "response_method": [
+            None,
+            list,
+            StrOptions({"predict", "predict_proba", "decision_function"}),
+        ],
     },
     prefer_skip_nested_validation=True,
 )
@@ -647,6 +664,7 @@ def make_scorer(
     greater_is_better=True,
     needs_proba=False,
     needs_threshold=False,
+    response_method=None,
     **kwargs,
 ):
     """Make a scorer from a performance metric or loss function.
@@ -696,6 +714,24 @@ def make_scorer(
         For example `average_precision` or the area under the roc curve
         can not be computed using discrete predictions alone.
 
+    response_method : {"predict_proba", "decision_function", "predict"} or \
+            list of such str, default=None
+
+        Specifies the response method to use get prediction from an estimator
+        (i.e. :term:`predict_proba`, :term:`decision_function` or
+        :term:`predict`). Possible choices are:
+
+        - if `str`, it corresponds to the name to the method to return;
+        - if a list of `str`, it provides the method names in order of
+          preference. The method returned corresponds to the first method in
+          the list and which is implemented by `estimator`.
+        - if `None`, the default order of methods is
+          `["predict_proba", "decision_function"]`.
+
+        Only used when `needs_threshold=True`.
+
+        .. versionadded:: 1.4
+
     **kwargs : additional arguments
         Additional parameters to be passed to `score_func`.
 
@@ -731,10 +767,11 @@ def make_scorer(
         raise ValueError(
             "Set either needs_proba or needs_threshold to True, but not both."
         )
-    if needs_proba:
-        cls = _ProbaScorer
-    elif needs_threshold:
+    if needs_threshold:
         cls = _ThresholdScorer
+        return cls(score_func, sign, kwargs, response_method)
+    elif needs_proba:
+        cls = _ProbaScorer
     else:
         cls = _PredictScorer
     return cls(score_func, sign, kwargs)
diff --git a/sklearn/metrics/tests/test_score_objects.py b/sklearn/metrics/tests/test_score_objects.py
index ffcb0ed4fda10..807265b206263 100644
--- a/sklearn/metrics/tests/test_score_objects.py
+++ b/sklearn/metrics/tests/test_score_objects.py
@@ -52,6 +52,7 @@
 )
 from sklearn.model_selection import GridSearchCV, cross_val_score, train_test_split
 from sklearn.multiclass import OneVsRestClassifier
+from sklearn.multioutput import ClassifierChain
 from sklearn.neighbors import KNeighborsClassifier
 from sklearn.pipeline import make_pipeline
 from sklearn.svm import LinearSVC
@@ -541,12 +542,15 @@ def test_thresholded_scorers_multilabel_indicator_data():
 
     # Multi-output multi-class decision_function
     # TODO Is there any yet?
-    clf = DecisionTreeClassifier()
-    clf.fit(X_train, y_train)
-    clf._predict_proba = clf.predict_proba
-    clf.predict_proba = None
-    clf.decision_function = lambda X: [p[:, 1] for p in clf._predict_proba(X)]
+    class TreeWithDecisionFunction(DecisionTreeClassifier):
+        # disable predict_proba
+        predict_proba = None
+
+        def decision_function(self, X):
+            return [p[:, 1] for p in DecisionTreeClassifier.predict_proba(self, X)]
 
+    clf = TreeWithDecisionFunction()
+    clf.fit(X_train, y_train)
     y_proba = clf.decision_function(X_test)
     score1 = get_scorer("roc_auc")(clf, X_test, y_test)
     score2 = roc_auc_score(y_test, np.vstack([p for p in y_proba]).T)
@@ -799,7 +803,23 @@ def test_multimetric_scorer_calls_method_once(
     assert decision_function_func.call_count == expected_decision_func_count
 
 
-def test_multimetric_scorer_calls_method_once_classifier_no_decision():
+@pytest.mark.parametrize(
+    "scorers",
+    [
+        (["roc_auc", "neg_log_loss"]),
+        (
+            {
+                "roc_auc": make_scorer(
+                    roc_auc_score,
+                    needs_threshold=True,
+                    response_method=["predict_proba", "decision_function"],
+                ),
+                "neg_log_loss": make_scorer(log_loss, needs_proba=True),
+            }
+        ),
+    ],
+)
+def test_multimetric_scorer_calls_method_once_classifier_no_decision(scorers):
     predict_proba_call_cnt = 0
 
     class MockKNeighborsClassifier(KNeighborsClassifier):
@@ -814,7 +834,6 @@ def predict_proba(self, X):
     clf = MockKNeighborsClassifier(n_neighbors=1)
     clf.fit(X, y)
 
-    scorers = ["roc_auc", "neg_log_loss"]
     scorer_dict = _check_multimetric_scoring(clf, scorers)
     scorer = _MultimetricScorer(scorers=scorer_dict)
     scorer(clf, X, y)
@@ -837,7 +856,7 @@ def predict(self, X):
     clf = MockDecisionTreeRegressor()
     clf.fit(X, y)
 
-    scorers = {"neg_mse": "neg_mean_squared_error", "r2": "roc_auc"}
+    scorers = {"neg_mse": "neg_mean_squared_error", "r2": "r2"}
     scorer_dict = _check_multimetric_scoring(clf, scorers)
     scorer = _MultimetricScorer(scorers=scorer_dict)
     scorer(clf, X, y)
@@ -1346,3 +1365,20 @@ def score(y_true, y_pred, param=None):
     with config_context(enable_metadata_routing=False):
         with pytest.raises(ValueError, match="kwargs is only supported if"):
             scorer(clf, X, y, param="blah")
+
+
+def test_get_scorer_multilabel_indicator():
+    """Check that our scorer deal with multi-label indicator matrices.
+
+    Non-regression test for:
+    https://github.com/scikit-learn/scikit-learn/issues/26817
+    """
+    X, Y = make_multilabel_classification(n_samples=72, n_classes=3, random_state=0)
+    X_train, X_test, Y_train, Y_test = train_test_split(X, Y, random_state=0)
+
+    base_lr = LogisticRegression(solver="lbfgs", random_state=0)
+    chain = ClassifierChain(base_lr, order="random", random_state=0)
+    chain.fit(X_train, Y_train)
+
+    score = get_scorer("average_precision")(chain, X_test, Y_test)
+    assert score > 0.8
diff --git a/sklearn/utils/_response.py b/sklearn/utils/_response.py
index e753ced045e1e..45138394a09ef 100644
--- a/sklearn/utils/_response.py
+++ b/sklearn/utils/_response.py
@@ -72,15 +72,17 @@ def _get_response_values(
     if is_classifier(estimator):
         prediction_method = _check_response_method(estimator, response_method)
         classes = estimator.classes_
-        target_type = "binary" if len(classes) <= 2 else "multiclass"
-
-        if pos_label is not None and pos_label not in classes.tolist():
-            raise ValueError(
-                f"pos_label={pos_label} is not a valid label: It should be "
-                f"one of {classes}"
-            )
-        elif pos_label is None and target_type == "binary":
-            pos_label = pos_label if pos_label is not None else classes[-1]
+        if isinstance(classes, list):
+            target_type = "multilabel-indicator"
+        else:
+            target_type = "binary" if len(classes) <= 2 else "multiclass"
+            if pos_label is not None and pos_label not in classes.tolist():
+                raise ValueError(
+                    f"pos_label={pos_label} is not a valid label: It should be "
+                    f"one of {classes}"
+                )
+            elif pos_label is None and target_type == "binary":
+                pos_label = pos_label if pos_label is not None else classes[-1]
 
         y_pred = prediction_method(X)
         if prediction_method.__name__ == "predict_proba":
@@ -94,10 +96,14 @@ def _get_response_values(
                         "classifier with two classes."
                     )
                     raise ValueError(err_msg)
+            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
+                y_pred = np.vstack([p[:, -1] for p in y_pred]).T
         elif prediction_method.__name__ == "decision_function":
             if target_type == "binary":
                 if pos_label == classes[0]:
                     y_pred *= -1
+            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
+                y_pred = np.vstack([p for p in y_pred]).T
     else:  # estimator is a regressor
         if response_method != "predict":
             raise ValueError(
diff --git a/sklearn/utils/tests/test_response.py b/sklearn/utils/tests/test_response.py
index a67346e5697ec..c428256558b62 100644
--- a/sklearn/utils/tests/test_response.py
+++ b/sklearn/utils/tests/test_response.py
@@ -1,11 +1,17 @@
 import numpy as np
 import pytest
 
-from sklearn.datasets import load_iris, make_classification, make_regression
+from sklearn.datasets import (
+    load_iris,
+    make_classification,
+    make_multilabel_classification,
+    make_regression,
+)
 from sklearn.linear_model import (
     LinearRegression,
     LogisticRegression,
 )
+from sklearn.multioutput import ClassifierChain
 from sklearn.preprocessing import scale
 from sklearn.tree import DecisionTreeClassifier, DecisionTreeRegressor
 from sklearn.utils._mocking import _MockEstimatorOnOffPrediction
@@ -230,3 +236,25 @@ def test_get_response_values_multiclass(estimator, response_method):
     assert predictions.shape == (X.shape[0], len(estimator.classes_))
     if response_method == "predict_proba":
         assert np.logical_and(predictions >= 0, predictions <= 1).all()
+
+
+@pytest.mark.parametrize(
+    "response_method", ["predict_proba", "decision_function", "predict"]
+)
+def test_get_response_values_multilabel_indicator(response_method):
+    X, Y = make_multilabel_classification(random_state=0)
+    estimator = ClassifierChain(LogisticRegression()).fit(X, Y)
+
+    y_pred, pos_label = _get_response_values(
+        estimator, X, response_method=response_method
+    )
+    assert pos_label is None
+    assert y_pred.shape == Y.shape
+
+    if response_method == "predict_proba":
+        assert np.logical_and(y_pred >= 0, y_pred <= 1).all()
+    elif response_method == "decision_function":
+        assert (y_pred < 0).sum() > 0
+        assert (y_pred > 0).sum() > 0
+    else:  # response_method == "predict"
+        assert np.logical_or(y_pred == 0, y_pred == 1).all()


Code review comments:
diff_hunk: @@ -452,20 +469,15 @@ def _score(self, method_caller, clf, X, y, **kwargs):
             raise ValueError("{0} format is not supported".format(y_type))
 
         if is_regressor(clf):
-            y_pred = method_caller(clf, "predict", X)
+            response_method = self._response_method or "predict"
path: sklearn/metrics/_scorer.py
text: @user1:
nit: What do you think of making this a multi-liner like a few lines below for the classifier case? For me it would make the symmetry clearer/less easy to overlook that `or "predict"`.

@author:
fair enough

diff_hunk: @@ -130,7 +129,21 @@ def __call__(self, estimator, *args, **kwargs):
                 **{name: Bunch(score=kwargs) for name in self._scorers}
             )
 
-        for name, scorer in self._scorers.items():
+        # to have the highest cache hit rate, we need to check if we have any
+        # _ThresholdScorer and choose a single response method based on the current
+        # estimator
+        scorers = copy.deepcopy(self._scorers)
path: sklearn/metrics/_scorer.py
text: @user1:
Why do we need to deepcopy the `_scorers` dict? My two initial reactions when seeing `deepcopy` are: (1) surely this isn't needed because you need the `copy` module very rarely and (2) you better explain why you need to do something weird like using `deepcopy`

@author:
We never mutate arguments and since `check_scoring` is not making a copy, we would mutate the argument passed by the user here. This is the reason for making a deep copy.

@user2:
I feel like there is more copying than there needs to be. I think we can do:

```python
def process_scorer(scorer):
    if not isinstance(scorer, _ThresholdScorer):
        return scorer
    if scorer._response_method is None:
        response_method = ("decision_function", "predict_proba")
    else:
        response_method = scorer._response_method
    new_response_method = _check_response_method(
        estimator, response_method).__name__

    # no need for deep copy
    new_scorer = scorer.copy()
    new_scorer._response_method = new_response_method
    return new_scorer

scorers = {name: process_scorer(scorer) name, scorer in self._scorers.items():
```

diff_hunk: @@ -72,15 +72,17 @@ def _get_response_values(
     if is_classifier(estimator):
         prediction_method = _check_response_method(estimator, response_method)
         classes = estimator.classes_
-        target_type = "binary" if len(classes) <= 2 else "multiclass"
-
-        if pos_label is not None and pos_label not in classes.tolist():
-            raise ValueError(
-                f"pos_label={pos_label} is not a valid label: It should be "
-                f"one of {classes}"
-            )
-        elif pos_label is None and target_type == "binary":
-            pos_label = pos_label if pos_label is not None else classes[-1]
+        if isinstance(classes, list):
+            target_type = "multilabel-indicator"
path: sklearn/utils/_response.py
text: @user1:
Can't we use `type_of_target(classes)` instead of having our own home-brew solution on this line and the ones below? Seems like it would be easier to understand by readers from the future than having these `if` statements and the implicit knowledge of why they work.

I'm thinking of something like:
```python
target_type = type_of_target(classes)
if target_type in ("multiclass", "binary"):
  if pos_label is not None and pos_label not in classes.tolist():
      raise ValueError(
          f"pos_label={pos_label} is not a valid label: It should be "
          f"one of {classes}"
      )
  elif pos_label is None and target_type == "binary":
      pos_label = pos_label if pos_label is not None else classes[-1]
```

@author:
For performance reasons, @user2 did not want to call it the first time. It involves calling `np.unique` and since we already have `classes_` from the estimator, then we don't need to.

@user1:
Isn't `classes_` going to be super small? Even a problem with a lot of classes will have less than a 100 unique classes no? Performance might not be a problem here.

@author:
The unique operation is on the full `y`.

@user2:
Although I do not like it, running `type_of_target(classes)` seems okay.

@author:
indeed `type_of_target` only on the classes would avoid spending to much time on the unique.