From 12e162754f1ede4a16d8117535174b7bb519ce79 Mon Sep 17 00:00:00 2001 From: genvalen Date: Mon, 8 Nov 2021 23:31:55 -0500 Subject: [PATCH 1/9] Add tests --- .../ensemble/tests/test_weight_boosting.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index 290f4b64dcc71..a5834cfebf221 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -569,6 +569,33 @@ def test_adaboost_classifier_params_validation(params, err_type, err_msg): AdaBoostClassifier(**params).fit(X, y_class) +@pytest.mark.parametrize( + # Test that it gives proper exception on deficient input. + "params, err_type, err_msg", + [ + ({"n_estimators": -1}, ValueError, "n_estimators == -1, must be >= 1"), + ({"n_estimators": 0}, ValueError, "n_estimators == 0, must be >= 1"), + ( + {"n_estimators": 1.5}, + TypeError, + "n_estimators must be an instance of ," + " not ", + ), + ({"learning_rate": -1}, ValueError, "learning_rate == -1, must be > 0."), + ({"learning_rate": 0}, ValueError, "learning_rate == 0, must be > 0."), + ( + {"loss": "unknown"}, + ValueError, + "loss must be 'linear', 'square', or 'exponential'", + ), + ], +) +def test_adaboost_regressor_params_validation(params, err_type, err_msg): + """Check the parameters validation in `AdaBoostRegressor`.""" + with pytest.raises(err_type, match=err_msg): + AdaBoostRegressor(**params).fit(X, y_regr) + + @pytest.mark.parametrize("algorithm", ["SAMME", "SAMME.R"]) def test_adaboost_consistent_predict(algorithm): # check that predict_proba and predict give consistent results From 8482313747366c4769342829a00503dc44dd3957 Mon Sep 17 00:00:00 2001 From: genvalen Date: Tue, 9 Nov 2021 00:04:01 -0500 Subject: [PATCH 2/9] Validate scalar parameters with check_scalar --- sklearn/ensemble/_weight_boosting.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/sklearn/ensemble/_weight_boosting.py b/sklearn/ensemble/_weight_boosting.py index ed0202a9a97ee..291a56efd2172 100644 --- a/sklearn/ensemble/_weight_boosting.py +++ b/sklearn/ensemble/_weight_boosting.py @@ -1078,9 +1078,29 @@ def fit(self, X, y, sample_weight=None): self : object Fitted AdaBoostRegressor estimator. """ + # Validate scalar parameters + check_scalar( + self.n_estimators, + "n_estimators", + target_type=numbers.Integral, + min_val=1, + include_boundaries="left", + ) + + check_scalar( + self.learning_rate, + "learning_rate", + target_type=numbers.Real, + min_val=0, + include_boundaries="neither", + ) + # Check loss if self.loss not in ("linear", "square", "exponential"): - raise ValueError("loss must be 'linear', 'square', or 'exponential'") + raise ValueError( + "loss must be 'linear', 'square', or 'exponential'" + f" Got {self.loss!r} instead." + ) # Fit return super().fit(X, y, sample_weight) From 9b8a673c21774a13c66c44c00d09aa1a64478115 Mon Sep 17 00:00:00 2001 From: genvalen Date: Wed, 24 Nov 2021 17:23:45 -0500 Subject: [PATCH 3/9] Move checks from AdaBoostRegressor to BaseWeightBoosting --- sklearn/ensemble/_weight_boosting.py | 36 +++++++++++++--------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/sklearn/ensemble/_weight_boosting.py b/sklearn/ensemble/_weight_boosting.py index 291a56efd2172..a050758af83a3 100644 --- a/sklearn/ensemble/_weight_boosting.py +++ b/sklearn/ensemble/_weight_boosting.py @@ -111,9 +111,22 @@ def fit(self, X, y, sample_weight=None): ------- self : object """ - # Check parameters - if self.learning_rate <= 0: - raise ValueError("learning_rate must be greater than zero") + # Validate scalar parameters + check_scalar( + self.n_estimators, + "n_estimators", + target_type=numbers.Integral, + min_val=1, + include_boundaries="left", + ) + + check_scalar( + self.learning_rate, + "learning_rate", + target_type=numbers.Real, + min_val=0, + include_boundaries="neither", + ) X, y = self._validate_data( X, @@ -1078,23 +1091,6 @@ def fit(self, X, y, sample_weight=None): self : object Fitted AdaBoostRegressor estimator. """ - # Validate scalar parameters - check_scalar( - self.n_estimators, - "n_estimators", - target_type=numbers.Integral, - min_val=1, - include_boundaries="left", - ) - - check_scalar( - self.learning_rate, - "learning_rate", - target_type=numbers.Real, - min_val=0, - include_boundaries="neither", - ) - # Check loss if self.loss not in ("linear", "square", "exponential"): raise ValueError( From 28e2af06ad24de28d70bed98b73bee3c0e67dd68 Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 10 Dec 2021 17:38:43 -0500 Subject: [PATCH 4/9] Update tests --- .../ensemble/tests/test_weight_boosting.py | 45 ++++++------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index a5834cfebf221..f74fcc9c59116 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -273,6 +273,12 @@ def test_importances(): def test_error(): # Test that it gives proper exception on deficient input. + with pytest.raises(ValueError): + AdaBoostRegressor(loss="foo").fit(X, y_class) + + with pytest.raises(ValueError): + AdaBoostClassifier(algorithm="foo").fit(X, y_class) + with pytest.raises(ValueError): AdaBoostClassifier().fit(X, y_class, sample_weight=np.asarray([-1])) @@ -542,7 +548,7 @@ def test_adaboostregressor_sample_weight(): assert score_with_outlier < score_with_weight assert score_no_outlier == pytest.approx(score_with_weight) - +# Test that it gives proper exception on deficient input. @pytest.mark.parametrize( "params, err_type, err_msg", [ @@ -556,44 +562,19 @@ def test_adaboostregressor_sample_weight(): ), ({"learning_rate": -1}, ValueError, "learning_rate == -1, must be > 0."), ({"learning_rate": 0}, ValueError, "learning_rate == 0, must be > 0."), - ( - {"algorithm": "unknown"}, - ValueError, - "Algorithm must be 'SAMME' or 'SAMME.R'.", - ), ], ) -def test_adaboost_classifier_params_validation(params, err_type, err_msg): - """Check the parameters validation in `AdaBoostClassifier`.""" - with pytest.raises(err_type, match=err_msg): - AdaBoostClassifier(**params).fit(X, y_class) - - @pytest.mark.parametrize( - # Test that it gives proper exception on deficient input. - "params, err_type, err_msg", + "model, X, y", [ - ({"n_estimators": -1}, ValueError, "n_estimators == -1, must be >= 1"), - ({"n_estimators": 0}, ValueError, "n_estimators == 0, must be >= 1"), - ( - {"n_estimators": 1.5}, - TypeError, - "n_estimators must be an instance of ," - " not ", - ), - ({"learning_rate": -1}, ValueError, "learning_rate == -1, must be > 0."), - ({"learning_rate": 0}, ValueError, "learning_rate == 0, must be > 0."), - ( - {"loss": "unknown"}, - ValueError, - "loss must be 'linear', 'square', or 'exponential'", - ), + (AdaBoostClassifier, X, y_class), + (AdaBoostRegressor, X, y_regr), ], ) -def test_adaboost_regressor_params_validation(params, err_type, err_msg): - """Check the parameters validation in `AdaBoostRegressor`.""" +def test_adaboost_params_validation(model, X, y, params, err_type, err_msg): + """Check input parameter validation in weight boosting.""" with pytest.raises(err_type, match=err_msg): - AdaBoostRegressor(**params).fit(X, y_regr) + model(**params).fit(X, y) @pytest.mark.parametrize("algorithm", ["SAMME", "SAMME.R"]) From 0335ea2177749e79117ec952bb1a14417aee938a Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 10 Dec 2021 17:41:15 -0500 Subject: [PATCH 5/9] Remove redundant checks from AdaBoostClassifer --- sklearn/ensemble/_weight_boosting.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/sklearn/ensemble/_weight_boosting.py b/sklearn/ensemble/_weight_boosting.py index a050758af83a3..05faa2535bc42 100644 --- a/sklearn/ensemble/_weight_boosting.py +++ b/sklearn/ensemble/_weight_boosting.py @@ -493,22 +493,6 @@ def fit(self, X, y, sample_weight=None): self : object Fitted estimator. """ - check_scalar( - self.n_estimators, - "n_estimators", - target_type=numbers.Integral, - min_val=1, - include_boundaries="left", - ) - - check_scalar( - self.learning_rate, - "learning_rate", - target_type=numbers.Real, - min_val=0, - include_boundaries="neither", - ) - # Check that algorithm is supported if self.algorithm not in ("SAMME", "SAMME.R"): raise ValueError( From 2cabe565e324633770cdf0cfc12e020fcbf30a76 Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 10 Dec 2021 17:46:46 -0500 Subject: [PATCH 6/9] Minor clean up and format with black --- sklearn/ensemble/tests/test_weight_boosting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index f74fcc9c59116..170199b5d6e19 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -548,7 +548,7 @@ def test_adaboostregressor_sample_weight(): assert score_with_outlier < score_with_weight assert score_no_outlier == pytest.approx(score_with_weight) -# Test that it gives proper exception on deficient input. + @pytest.mark.parametrize( "params, err_type, err_msg", [ From 563c5f002bc312790010c9fe4ff8d862cc83f5d0 Mon Sep 17 00:00:00 2001 From: genvalen Date: Tue, 14 Dec 2021 17:27:57 -0500 Subject: [PATCH 7/9] Update sklearn/ensemble/tests/test_weight_boosting.py Co-authored-by: Thomas J. Fan --- sklearn/ensemble/tests/test_weight_boosting.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index 170199b5d6e19..707f563345967 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -276,8 +276,9 @@ def test_error(): with pytest.raises(ValueError): AdaBoostRegressor(loss="foo").fit(X, y_class) + clf = AdaBoostClassifier(algorithm="foo") with pytest.raises(ValueError): - AdaBoostClassifier(algorithm="foo").fit(X, y_class) + clf.fit(X, y_class) with pytest.raises(ValueError): AdaBoostClassifier().fit(X, y_class, sample_weight=np.asarray([-1])) From 2ae607d7cb31868238f5be8138577825af21fd62 Mon Sep 17 00:00:00 2001 From: genvalen Date: Tue, 14 Dec 2021 17:28:33 -0500 Subject: [PATCH 8/9] Update sklearn/ensemble/tests/test_weight_boosting.py Co-authored-by: Thomas J. Fan --- sklearn/ensemble/tests/test_weight_boosting.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index 707f563345967..1ec124d1085b4 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -273,8 +273,9 @@ def test_importances(): def test_error(): # Test that it gives proper exception on deficient input. + reg = AdaBoostRegressor(loss="foo") with pytest.raises(ValueError): - AdaBoostRegressor(loss="foo").fit(X, y_class) + reg.fit(X, y_class) clf = AdaBoostClassifier(algorithm="foo") with pytest.raises(ValueError): From 360e7d08df6d6092f650d3de2431140f37812ea6 Mon Sep 17 00:00:00 2001 From: genvalen Date: Tue, 14 Dec 2021 17:35:42 -0500 Subject: [PATCH 9/9] Ensure validation happens in --- sklearn/ensemble/tests/test_weight_boosting.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index 1ec124d1085b4..91a537257ac65 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -575,8 +575,9 @@ def test_adaboostregressor_sample_weight(): ) def test_adaboost_params_validation(model, X, y, params, err_type, err_msg): """Check input parameter validation in weight boosting.""" + est = model(**params) with pytest.raises(err_type, match=err_msg): - model(**params).fit(X, y) + est.fit(X, y) @pytest.mark.parametrize("algorithm", ["SAMME", "SAMME.R"])