From b44dd9d617a6174ef02f6744b9286b0922659273 Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Fri, 31 Mar 2023 17:14:44 +0200 Subject: [PATCH 1/9] MAINT refactor scorer using _get_response_values --- sklearn/metrics/_scorer.py | 94 +++++++-------------- sklearn/metrics/tests/test_score_objects.py | 3 +- sklearn/utils/_response.py | 10 +-- sklearn/utils/tests/test_response.py | 20 ----- 4 files changed, 38 insertions(+), 89 deletions(-) diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py index 9df1b482bdeb3..f10e398debcd6 100644 --- a/sklearn/metrics/_scorer.py +++ b/sklearn/metrics/_scorer.py @@ -18,9 +18,10 @@ # Arnaud Joly # License: Simplified BSD +from collections import Counter from collections.abc import Iterable +from inspect import signature from functools import partial -from collections import Counter from traceback import format_exc import numpy as np @@ -65,19 +66,21 @@ from ..utils.multiclass import type_of_target from ..base import is_regressor +from ..utils._response import _get_response_values from ..utils._param_validation import validate_params -def _cached_call(cache, estimator, method, *args, **kwargs): +def _cached_call(cache, estimator, *args, **kwargs): """Call estimator with method and args and kwargs.""" if cache is None: - return getattr(estimator, method)(*args, **kwargs) + return _get_response_values(estimator, *args, **kwargs) + response_method = kwargs["response_method"] try: - return cache[method] + return cache[response_method] except KeyError: - result = getattr(estimator, method)(*args, **kwargs) - cache[method] = result + result = _get_response_values(estimator, *args, **kwargs) + cache[response_method] = result return result @@ -163,40 +166,15 @@ def __init__(self, score_func, sign, kwargs): self._score_func = score_func self._sign = sign - @staticmethod - def _check_pos_label(pos_label, classes): - if pos_label not in list(classes): - raise ValueError(f"pos_label={pos_label} is not a valid label: {classes}") - - def _select_proba_binary(self, y_pred, classes): - """Select the column of the positive label in `y_pred` when - probabilities are provided. - - Parameters - ---------- - y_pred : ndarray of shape (n_samples, n_classes) - The prediction given by `predict_proba`. - - classes : ndarray of shape (n_classes,) - The class labels for the estimator. - - Returns - ------- - y_pred : ndarray of shape (n_samples,) - Probability predictions of the positive class. - """ - if y_pred.shape[1] == 2: - pos_label = self._kwargs.get("pos_label", classes[1]) - self._check_pos_label(pos_label, classes) - col_idx = np.flatnonzero(classes == pos_label)[0] - return y_pred[:, col_idx] - - err_msg = ( - f"Got predict_proba of shape {y_pred.shape}, but need " - f"classifier with two classes for {self._score_func.__name__} " - "scoring" - ) - raise ValueError(err_msg) + def _get_pos_label(self): + score_func_params = signature(self._score_func).parameters + if "pos_label" in self._kwargs: + pos_label = self._kwargs["pos_label"] + elif "pos_label" in score_func_params: + pos_label = score_func_params["pos_label"].default + else: + pos_label = None + return pos_label def __repr__(self): kwargs_string = "".join( @@ -274,7 +252,7 @@ def _score(self, method_caller, estimator, X, y_true, sample_weight=None): Score function applied to prediction of estimator on X. """ - y_pred = method_caller(estimator, "predict", X) + y_pred, _ = method_caller(estimator, X, response_method="predict") if sample_weight is not None: return self._sign * self._score_func( y_true, y_pred, sample_weight=sample_weight, **self._kwargs @@ -312,14 +290,9 @@ def _score(self, method_caller, clf, X, y, sample_weight=None): score : float Score function applied to prediction of estimator on X. """ - - y_type = type_of_target(y) - y_pred = method_caller(clf, "predict_proba", X) - if y_type == "binary" and y_pred.shape[1] <= 2: - # `y_type` could be equal to "binary" even in a multi-class - # problem: (when only 2 class are given to `y_true` during scoring) - # Thus, we need to check for the shape of `y_pred`. - y_pred = self._select_proba_binary(y_pred, clf.classes_) + y_pred, _ = method_caller( + clf, X, response_method="predict_proba", pos_label=self._get_pos_label() + ) if sample_weight is not None: return self._sign * self._score_func( y, y_pred, sample_weight=sample_weight, **self._kwargs @@ -368,28 +341,23 @@ def _score(self, method_caller, clf, X, y, sample_weight=None): raise ValueError("{0} format is not supported".format(y_type)) if is_regressor(clf): - y_pred = method_caller(clf, "predict", X) + y_pred, _ = method_caller(clf, X, response_method="predict") else: + pos_label = self._get_pos_label() try: - y_pred = method_caller(clf, "decision_function", X) + y_pred, _ = method_caller( + clf, X, response_method="decision_function", 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 - elif y_type == "binary" and "pos_label" in self._kwargs: - self._check_pos_label(self._kwargs["pos_label"], clf.classes_) - if self._kwargs["pos_label"] == clf.classes_[0]: - # The implicit positive class of the binary classifier - # does not match `pos_label`: we need to invert the - # predictions - y_pred *= -1 except (NotImplementedError, AttributeError): - y_pred = method_caller(clf, "predict_proba", X) - - if y_type == "binary": - y_pred = self._select_proba_binary(y_pred, clf.classes_) - elif isinstance(y_pred, list): + y_pred, _ = method_caller( + clf, X, response_method="predict_proba", pos_label=pos_label + ) + if isinstance(y_pred, list): y_pred = np.vstack([p[:, -1] for p in y_pred]).T if sample_weight is not None: diff --git a/sklearn/metrics/tests/test_score_objects.py b/sklearn/metrics/tests/test_score_objects.py index d39db7fc894c4..b0186c42f8921 100644 --- a/sklearn/metrics/tests/test_score_objects.py +++ b/sklearn/metrics/tests/test_score_objects.py @@ -770,6 +770,7 @@ def test_multimetric_scorer_calls_method_once( X, y = np.array([[1], [1], [0], [0], [0]]), np.array([0, 1, 1, 1, 0]) mock_est = Mock() + mock_est._estimator_type = "classifier" fit_func = Mock(return_value=mock_est) predict_func = Mock(return_value=y) @@ -972,7 +973,7 @@ def test_multiclass_roc_no_proba_scorer_errors(scorer_name): n_classes=3, n_informative=3, n_samples=20, random_state=0 ) lr = Perceptron().fit(X, y) - msg = "'Perceptron' object has no attribute 'predict_proba'" + msg = "Perceptron has none of the following attributes: predict_proba." with pytest.raises(AttributeError, match=msg): scorer(lr, X, y) diff --git a/sklearn/utils/_response.py b/sklearn/utils/_response.py index 50b9409c8276d..e11a67cca6d56 100644 --- a/sklearn/utils/_response.py +++ b/sklearn/utils/_response.py @@ -78,11 +78,11 @@ def _get_response_values( target_type = "binary" if len(classes) <= 2 else "multiclass" - if target_type == "multiclass" and prediction_method.__name__ != "predict": - raise ValueError( - "With a multiclass estimator, the response method should be " - f"predict, got {prediction_method.__name__} instead." - ) + # if target_type == "multiclass" and prediction_method.__name__ != "predict": + # raise ValueError( + # "With a multiclass estimator, the response method should be " + # f"predict, got {prediction_method.__name__} instead." + # ) if pos_label is not None and pos_label not in classes.tolist(): raise ValueError( diff --git a/sklearn/utils/tests/test_response.py b/sklearn/utils/tests/test_response.py index 0e2ce5fe5f038..9d6d90ddd94ae 100644 --- a/sklearn/utils/tests/test_response.py +++ b/sklearn/utils/tests/test_response.py @@ -6,7 +6,6 @@ LinearRegression, LogisticRegression, ) -from sklearn.svm import SVC from sklearn.tree import DecisionTreeClassifier, DecisionTreeRegressor from sklearn.utils._mocking import _MockEstimatorOnOffPrediction from sklearn.utils._testing import assert_allclose, assert_array_equal @@ -29,25 +28,6 @@ def test_get_response_values_regressor_error(response_method): _get_response_values(my_estimator, X, response_method=response_method) -@pytest.mark.parametrize( - "estimator, response_method", - [ - (DecisionTreeClassifier(), "predict_proba"), - (SVC(), "decision_function"), - ], -) -def test_get_response_values_error_multiclass_classifier(estimator, response_method): - """Check that we raise an error with multiclass classifier and requesting - response values different from `predict`.""" - X, y = make_classification( - n_samples=10, n_clusters_per_class=1, n_classes=3, random_state=0 - ) - classifier = estimator.fit(X, y) - err_msg = "With a multiclass estimator, the response method should be predict" - with pytest.raises(ValueError, match=err_msg): - _get_response_values(classifier, X, response_method=response_method) - - def test_get_response_values_regressor(): """Check the behaviour of `_get_response_values` with regressor.""" X, y = make_regression(n_samples=10, random_state=0) From 516f62f887fc67101ec2c6edb43dfecb39fc8e5a Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Sat, 1 Apr 2023 15:51:30 +0200 Subject: [PATCH 2/9] Add __name__ for method of Mock --- sklearn/metrics/tests/test_score_objects.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sklearn/metrics/tests/test_score_objects.py b/sklearn/metrics/tests/test_score_objects.py index b0186c42f8921..a2d47e1b81a2a 100644 --- a/sklearn/metrics/tests/test_score_objects.py +++ b/sklearn/metrics/tests/test_score_objects.py @@ -771,13 +771,17 @@ def test_multimetric_scorer_calls_method_once( mock_est = Mock() mock_est._estimator_type = "classifier" - fit_func = Mock(return_value=mock_est) - predict_func = Mock(return_value=y) + fit_func = Mock(return_value=mock_est, name="fit") + fit_func.__name__ = "fit" + predict_func = Mock(return_value=y, name="predict") + predict_func.__name__ = "predict" pos_proba = np.random.rand(X.shape[0]) proba = np.c_[1 - pos_proba, pos_proba] - predict_proba_func = Mock(return_value=proba) - decision_function_func = Mock(return_value=pos_proba) + predict_proba_func = Mock(return_value=proba, name="predict_proba") + predict_proba_func.__name__ = "predict_proba" + decision_function_func = Mock(return_value=pos_proba, name="decision_function") + decision_function_func.__name__ = "decision_function" mock_est.fit = fit_func mock_est.predict = predict_func From d2fbee0235a3380b6eb49df31bbf9e8d210eb4d5 Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Sat, 1 Apr 2023 15:52:34 +0200 Subject: [PATCH 3/9] remove multiclass issue --- sklearn/utils/_response.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/sklearn/utils/_response.py b/sklearn/utils/_response.py index e11a67cca6d56..e92b167b241de 100644 --- a/sklearn/utils/_response.py +++ b/sklearn/utils/_response.py @@ -19,9 +19,6 @@ def _get_response_values( The response values are predictions, one scalar value for each sample in X that depends on the specific choice of `response_method`. - This helper only accepts multiclass classifiers with the `predict` response - method. - If `estimator` is a binary classifier, also return the label for the effective positive class. @@ -75,15 +72,8 @@ 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 target_type == "multiclass" and prediction_method.__name__ != "predict": - # raise ValueError( - # "With a multiclass estimator, the response method should be " - # f"predict, got {prediction_method.__name__} instead." - # ) - 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 " From 29e5e876b2744d229e5448ee1246fcf19a4ec99f Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Mon, 3 Apr 2023 10:22:47 +0200 Subject: [PATCH 4/9] make response_method a mandatory arg --- sklearn/metrics/_scorer.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py index f10e398debcd6..bd93d1bd80e3c 100644 --- a/sklearn/metrics/_scorer.py +++ b/sklearn/metrics/_scorer.py @@ -70,16 +70,18 @@ from ..utils._param_validation import validate_params -def _cached_call(cache, estimator, *args, **kwargs): +def _cached_call(cache, estimator, response_method, *args, **kwargs): """Call estimator with method and args and kwargs.""" if cache is None: - return _get_response_values(estimator, *args, **kwargs) - - response_method = kwargs["response_method"] + return _get_response_values( + estimator, *args, response_method=response_method, **kwargs + ) try: return cache[response_method] except KeyError: - result = _get_response_values(estimator, *args, **kwargs) + result = _get_response_values( + estimator, *args, response_method=response_method, **kwargs + ) cache[response_method] = result return result @@ -252,7 +254,7 @@ def _score(self, method_caller, estimator, X, y_true, sample_weight=None): Score function applied to prediction of estimator on X. """ - y_pred, _ = method_caller(estimator, X, response_method="predict") + y_pred, _ = method_caller(estimator, "predict", X) if sample_weight is not None: return self._sign * self._score_func( y_true, y_pred, sample_weight=sample_weight, **self._kwargs @@ -291,7 +293,7 @@ def _score(self, method_caller, clf, X, y, sample_weight=None): Score function applied to prediction of estimator on X. """ y_pred, _ = method_caller( - clf, X, response_method="predict_proba", pos_label=self._get_pos_label() + clf, "predict_proba", X, pos_label=self._get_pos_label() ) if sample_weight is not None: return self._sign * self._score_func( @@ -341,12 +343,12 @@ def _score(self, method_caller, clf, X, y, sample_weight=None): raise ValueError("{0} format is not supported".format(y_type)) if is_regressor(clf): - y_pred, _ = method_caller(clf, X, response_method="predict") + y_pred, _ = method_caller(clf, "predict", X) else: pos_label = self._get_pos_label() try: y_pred, _ = method_caller( - clf, X, response_method="decision_function", pos_label=pos_label + clf, "decision_function", X, pos_label=pos_label ) if isinstance(y_pred, list): @@ -354,9 +356,7 @@ def _score(self, method_caller, clf, X, y, sample_weight=None): y_pred = np.vstack([p for p in y_pred]).T except (NotImplementedError, AttributeError): - y_pred, _ = method_caller( - clf, X, response_method="predict_proba", pos_label=pos_label - ) + 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 From b645ade807103ad1c40dd4572250f9e5c4399a0d Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Mon, 3 Apr 2023 16:42:39 +0200 Subject: [PATCH 5/9] Update sklearn/metrics/_scorer.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com> --- sklearn/metrics/_scorer.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py index bd93d1bd80e3c..85eb50058542c 100644 --- a/sklearn/metrics/_scorer.py +++ b/sklearn/metrics/_scorer.py @@ -171,12 +171,10 @@ def __init__(self, score_func, sign, kwargs): def _get_pos_label(self): score_func_params = signature(self._score_func).parameters if "pos_label" in self._kwargs: - pos_label = self._kwargs["pos_label"] - elif "pos_label" in score_func_params: - pos_label = score_func_params["pos_label"].default - else: - pos_label = None - return pos_label + return self._kwargs["pos_label"] + if "pos_label" in score_func_params: + return score_func_params["pos_label"].default + return None def __repr__(self): kwargs_string = "".join( From 3397c5603e80e3be42ad5c57c6f971f8959de288 Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Mon, 3 Apr 2023 16:45:20 +0200 Subject: [PATCH 6/9] apply jeremie comments --- sklearn/metrics/_scorer.py | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py index 85eb50058542c..ab83be055ebe5 100644 --- a/sklearn/metrics/_scorer.py +++ b/sklearn/metrics/_scorer.py @@ -72,18 +72,17 @@ def _cached_call(cache, estimator, response_method, *args, **kwargs): """Call estimator with method and args and kwargs.""" - if cache is None: - return _get_response_values( - estimator, *args, response_method=response_method, **kwargs - ) - try: + if cache is not None and response_method in cache: return cache[response_method] - except KeyError: - result = _get_response_values( - estimator, *args, response_method=response_method, **kwargs - ) + + result, _ = _get_response_values( + estimator, *args, response_method=response_method, **kwargs + ) + + if cache is not None: cache[response_method] = result - return result + + return result class _MultimetricScorer: @@ -252,7 +251,7 @@ def _score(self, method_caller, estimator, X, y_true, sample_weight=None): Score function applied to prediction of estimator on X. """ - y_pred, _ = method_caller(estimator, "predict", X) + y_pred = method_caller(estimator, "predict", X) if sample_weight is not None: return self._sign * self._score_func( y_true, y_pred, sample_weight=sample_weight, **self._kwargs @@ -290,9 +289,7 @@ def _score(self, method_caller, clf, X, y, sample_weight=None): score : float Score function applied to prediction of estimator on X. """ - y_pred, _ = method_caller( - clf, "predict_proba", X, pos_label=self._get_pos_label() - ) + y_pred = method_caller(clf, "predict_proba", X, pos_label=self._get_pos_label()) if sample_weight is not None: return self._sign * self._score_func( y, y_pred, sample_weight=sample_weight, **self._kwargs @@ -341,20 +338,18 @@ def _score(self, method_caller, clf, X, y, sample_weight=None): raise ValueError("{0} format is not supported".format(y_type)) if is_regressor(clf): - y_pred, _ = method_caller(clf, "predict", X) + y_pred = method_caller(clf, "predict", X) else: pos_label = self._get_pos_label() try: - y_pred, _ = method_caller( - clf, "decision_function", X, pos_label=pos_label - ) + 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) + 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 From a33d995c2cac9ce19ca88dcbb40dd749055bc8c7 Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Mon, 24 Apr 2023 19:55:34 +0200 Subject: [PATCH 7/9] Update sklearn/metrics/_scorer.py Co-authored-by: Adrin Jalali --- sklearn/metrics/_scorer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py index ab83be055ebe5..df75ae6803cf7 100644 --- a/sklearn/metrics/_scorer.py +++ b/sklearn/metrics/_scorer.py @@ -168,9 +168,9 @@ def __init__(self, score_func, sign, kwargs): self._sign = sign def _get_pos_label(self): - score_func_params = signature(self._score_func).parameters if "pos_label" in self._kwargs: - return self._kwargs["pos_label"] + return self._kwargs["pos_label" + score_func_params = signature(self._score_func).parameters if "pos_label" in score_func_params: return score_func_params["pos_label"].default return None From 15e9c373ab46d64f86c16f74b7442b6322b1c165 Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Mon, 24 Apr 2023 19:56:30 +0200 Subject: [PATCH 8/9] Update sklearn/metrics/_scorer.py --- sklearn/metrics/_scorer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py index df75ae6803cf7..80f1ded4ced15 100644 --- a/sklearn/metrics/_scorer.py +++ b/sklearn/metrics/_scorer.py @@ -169,7 +169,7 @@ def __init__(self, score_func, sign, kwargs): def _get_pos_label(self): if "pos_label" in self._kwargs: - return self._kwargs["pos_label" + return self._kwargs["pos_label"] score_func_params = signature(self._score_func).parameters if "pos_label" in score_func_params: return score_func_params["pos_label"].default From 2aa8ab650981ce665cc65b377565efb5352ce81c Mon Sep 17 00:00:00 2001 From: Guillaume Lemaitre Date: Thu, 4 May 2023 10:48:50 +0200 Subject: [PATCH 9/9] TST add new test for the multiclass case --- sklearn/utils/tests/test_response.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/sklearn/utils/tests/test_response.py b/sklearn/utils/tests/test_response.py index 9d6d90ddd94ae..a0e7c30a81a77 100644 --- a/sklearn/utils/tests/test_response.py +++ b/sklearn/utils/tests/test_response.py @@ -6,6 +6,7 @@ LinearRegression, LogisticRegression, ) +from sklearn.preprocessing import scale from sklearn.tree import DecisionTreeClassifier, DecisionTreeRegressor from sklearn.utils._mocking import _MockEstimatorOnOffPrediction from sklearn.utils._testing import assert_allclose, assert_array_equal @@ -14,6 +15,8 @@ X, y = load_iris(return_X_y=True) +# scale the data to avoid ConvergenceWarning with LogisticRegression +X = scale(X, copy=False) X_binary, y_binary = X[:100], y[:100] @@ -207,3 +210,25 @@ def test_get_response_decision_function(): ) np.testing.assert_allclose(y_score, classifier.decision_function(X_binary) * -1) assert pos_label == 0 + + +@pytest.mark.parametrize( + "estimator, response_method", + [ + (DecisionTreeClassifier(max_depth=2, random_state=0), "predict_proba"), + (LogisticRegression(), "decision_function"), + ], +) +def test_get_response_values_multiclass(estimator, response_method): + """Check that we can call `_get_response_values` with a multiclass estimator. + It should return the predictions untouched. + """ + estimator.fit(X, y) + predictions, pos_label = _get_response_values( + estimator, X, response_method=response_method + ) + + assert pos_label is None + assert predictions.shape == (X.shape[0], len(estimator.classes_)) + if response_method == "predict_proba": + assert np.logical_and(predictions >= 0, predictions <= 1).all()