From 5fb1f500e467c3790db0453e71c2b1d9ff1af1ea Mon Sep 17 00:00:00 2001 From: adrinjalali Date: Tue, 14 Mar 2023 15:21:22 +0100 Subject: [PATCH] MNT remove backward compatibility from meta-estimators --- sklearn/calibration.py | 5 -- sklearn/metrics/_scorer.py | 11 +-- sklearn/multioutput.py | 30 +++----- .../test_metaestimators_metadata_routing.py | 72 ------------------- sklearn/tests/test_multioutput.py | 10 +-- 5 files changed, 11 insertions(+), 117 deletions(-) diff --git a/sklearn/calibration.py b/sklearn/calibration.py index ac7c3f3fa91e2..4daf7c7ed9577 100644 --- a/sklearn/calibration.py +++ b/sklearn/calibration.py @@ -511,11 +511,6 @@ def get_metadata_routing(self): splitter=self.cv, method_mapping=MethodMapping().add(callee="split", caller="fit"), ) - # the fit method already accepts everything, therefore we don't - # specify parameters. The value passed to ``child`` needs to be the - # same as what's passed to ``add`` above, in this case - # `"estimator"`. - .warn_on(child="estimator", method="fit", params=None) ) return router diff --git a/sklearn/metrics/_scorer.py b/sklearn/metrics/_scorer.py index 04baaf8859080..58d99a7c1b4ce 100644 --- a/sklearn/metrics/_scorer.py +++ b/sklearn/metrics/_scorer.py @@ -402,11 +402,6 @@ def _score(self, method_caller, clf, X, y, **kwargs): y_pred = self._select_proba_binary(y_pred, clf.classes_) scoring_kwargs = {**self._kwargs, **kwargs} - # this is for backward compatibility to avoid passing sample_weight - # to the scorer if it's None - # TODO(1.3) Probably remove - if scoring_kwargs.get("sample_weight", -1) is None: - del scoring_kwargs["sample_weight"] return self._sign * self._score_func(y, y_pred, **scoring_kwargs) @@ -487,11 +482,7 @@ def _score(self, method_caller, clf, X, y, **kwargs): y_pred = np.vstack([p[:, -1] for p in y_pred]).T scoring_kwargs = {**self._kwargs, **kwargs} - # this is for backward compatibility to avoid passing sample_weight - # to the scorer if it's None - # TODO(1.3) Probably remove - if scoring_kwargs.get("sample_weight", -1) is None: - del scoring_kwargs["sample_weight"] + return self._sign * self._score_func(y, y_pred, **scoring_kwargs) def _factory_args(self): diff --git a/sklearn/multioutput.py b/sklearn/multioutput.py index b09debf6b50b3..34f04ad19bb44 100644 --- a/sklearn/multioutput.py +++ b/sklearn/multioutput.py @@ -269,21 +269,11 @@ def get_metadata_routing(self): A :class:`~utils.metadata_routing.MetadataRouter` encapsulating routing information. """ - router = ( - MetadataRouter(owner=self.__class__.__name__).add( - estimator=self.estimator, - method_mapping=MethodMapping() - .add(callee="partial_fit", caller="partial_fit") - .add(callee="fit", caller="fit"), - ) - # the fit method already accepts everything, therefore we don't - # specify parameters. The value passed to ``child`` needs to be the - # same as what's passed to ``add`` above, in this case - # `"estimator"`. - .warn_on(child="estimator", method="fit", params=None) - # the partial_fit method at the time of this change (v1.2) only - # supports sample_weight, therefore we only include this metadata. - .warn_on(child="estimator", method="partial_fit", params=["sample_weight"]) + router = MetadataRouter(owner=self.__class__.__name__).add( + estimator=self.estimator, + method_mapping=MethodMapping() + .add(callee="partial_fit", caller="partial_fit") + .add(callee="fit", caller="fit"), ) return router @@ -1087,13 +1077,9 @@ def get_metadata_routing(self): A :class:`~utils.metadata_routing.MetadataRouter` encapsulating routing information. """ - router = ( - MetadataRouter(owner=self.__class__.__name__) - .add( - estimator=self.base_estimator, - method_mapping=MethodMapping().add(callee="fit", caller="fit"), - ) - .warn_on(child="estimator", method="fit", params=None) + router = MetadataRouter(owner=self.__class__.__name__).add( + estimator=self.base_estimator, + method_mapping=MethodMapping().add(callee="fit", caller="fit"), ) return router diff --git a/sklearn/tests/test_metaestimators_metadata_routing.py b/sklearn/tests/test_metaestimators_metadata_routing.py index 9cc21135cf225..2f8f850f3e6a3 100644 --- a/sklearn/tests/test_metaestimators_metadata_routing.py +++ b/sklearn/tests/test_metaestimators_metadata_routing.py @@ -16,7 +16,6 @@ from sklearn.utils.metadata_routing import MetadataRouter from sklearn.tests.test_metadata_routing import ( record_metadata, - check_recorded_metadata, assert_request_is_empty, ) @@ -159,10 +158,6 @@ def predict_log_proba(self, X, sample_weight="default", metadata="default"): "X": X, "y": y_multi, "routing_methods": ["fit", "partial_fit"], - "warns_on": { - "fit": ["sample_weight", "metadata"], - "partial_fit": ["sample_weight"], - }, }, { "metaestimator": MultiOutputClassifier, @@ -171,10 +166,6 @@ def predict_log_proba(self, X, sample_weight="default", metadata="default"): "X": X, "y": y_multi, "routing_methods": ["fit", "partial_fit"], - "warns_on": { - "fit": ["sample_weight", "metadata"], - "partial_fit": ["sample_weight"], - }, }, { "metaestimator": CalibratedClassifierCV, @@ -183,7 +174,6 @@ def predict_log_proba(self, X, sample_weight="default", metadata="default"): "X": X, "y": y, "routing_methods": ["fit"], - "warns_on": {"fit": ["sample_weight", "metadata"]}, "preserves_metadata": False, }, { @@ -193,7 +183,6 @@ def predict_log_proba(self, X, sample_weight="default", metadata="default"): "X": X, "y": y_multi, "routing_methods": ["fit"], - "warns_on": {}, }, { "metaestimator": RegressorChain, @@ -202,7 +191,6 @@ def predict_log_proba(self, X, sample_weight="default", metadata="default"): "X": X, "y": y_multi, "routing_methods": ["fit"], - "warns_on": {"fit": ["sample_weight", "metadata"]}, }, ] """List containing all metaestimators to be tested and their settings @@ -215,10 +203,6 @@ def predict_log_proba(self, X, sample_weight="default", metadata="default"): - X: X-data to fit and predict - y: y-data to fit - routing_methods: list of all methods to check for routing -- warns_on: A dict containing all methods as keys, and arguments as values, - whose combination is supposed to result in a warning if routing is not - requested. It is implied that all routing methods and arguments not listed - here should result in an error. - preserves_metadata: Whether the metaestimator passes the metadata to the sub-estimator without modification or not. If it does, we check that the values are identical. If it doesn', no check is performed. TODO Maybe @@ -245,56 +229,6 @@ def test_default_request(metaestimator): assert isinstance(instance.get_metadata_routing(), MetadataRouter) -@pytest.mark.parametrize( - "metaestimator", - METAESTIMATORS, - ids=METAESTIMATOR_IDS, -) -def test_warning_for_indicated_methods(metaestimator): - # Check that the indicated methods give a warning - # TODO: Always error for 1.4 - cls = metaestimator["metaestimator"] - registry = _Registry() - estimator = metaestimator["estimator"](registry=registry) - estimator_name = metaestimator["estimator_name"] - X = metaestimator["X"] - y = metaestimator["y"] - routing_methods = metaestimator["routing_methods"] - warns_on = metaestimator["warns_on"] - - for method_name in routing_methods: - if method_name not in warns_on: - # this method is not expected to warn - continue - - for key in warns_on[method_name]: - val = {"sample_weight": sample_weight, "metadata": metadata}[key] - kwargs = {key: val} - warn_msg = ( - "You are passing metadata for which the request values are not" - f" explicitly set: {key}. From version 1.4 this results in the" - f" following error: [{key}] are passed but are not explicitly set as" - f" requested or not for {estimator.__class__.__name__}.{method_name}" - ) - - instance = cls(**{estimator_name: estimator}) - if "fit" not in method_name: # instance needs to be fitted first - instance.fit(X, y) - with pytest.warns(FutureWarning, match=re.escape(warn_msg)): - method = getattr(instance, method_name) - method(X, y, **kwargs) - - if metaestimator.get("preserves_metadata", True): - # sanity check that registry is not empty, or else the test - # passes trivially - assert registry - for estimator in registry: - check_recorded_metadata(estimator, method_name, **kwargs) - # clear the registry since the check could be different for the next - # method being tested - registry.clear() - - @pytest.mark.parametrize( "metaestimator", METAESTIMATORS, @@ -309,15 +243,9 @@ def test_error_for_other_methods(metaestimator): X = metaestimator["X"] y = metaestimator["y"] routing_methods = metaestimator["routing_methods"] - warns_on = metaestimator["warns_on"] for method_name in routing_methods: - warn_args = warns_on.get(method_name, []) for key in ["sample_weight", "metadata"]: - if key in warn_args: - # this method is expected to warn for this argument, not raise - continue - val = {"sample_weight": sample_weight, "metadata": metadata}[key] kwargs = {key: val} msg = ( diff --git a/sklearn/tests/test_multioutput.py b/sklearn/tests/test_multioutput.py index 416d82a1b47e9..f582892fc22e8 100644 --- a/sklearn/tests/test_multioutput.py +++ b/sklearn/tests/test_multioutput.py @@ -656,15 +656,9 @@ def fit(self, X, y, **fit_params): for est in model.estimators_: assert est.sample_weight_ is weight - # TODO(1.4): Remove check for FutureWarning - # Test that the existing behavior works and raises a FutureWarning - # when the underlying estimator used has a sample_weight parameter - # defined in it's fit method. - model = RegressorChain(QuantileRegressor()) + model = RegressorChain(QuantileRegressor().set_fit_request(sample_weight=True)) fit_param = {"sample_weight": weight} - - with pytest.warns(FutureWarning): - model.fit(X, y, **fit_param) + model.fit(X, y, **fit_param) @pytest.mark.parametrize(