From 6c1abc356f5f60077827253a5950e57663ead288 Mon Sep 17 00:00:00 2001 From: genvalen Date: Wed, 12 Jan 2022 13:28:28 -0500 Subject: [PATCH 01/13] Create PR --- sklearn/ensemble/_voting.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sklearn/ensemble/_voting.py b/sklearn/ensemble/_voting.py index 99591cedead3d..1f6d5508a96ff 100644 --- a/sklearn/ensemble/_voting.py +++ b/sklearn/ensemble/_voting.py @@ -62,6 +62,7 @@ def _predict(self, X): @abstractmethod def fit(self, X, y, sample_weight=None): """Get common fit operations.""" + # check scalar parameters names, clfs = self._validate_estimators() if self.weights is not None and len(self.weights) != len(self.estimators): From 9993930fd6b17737ac991eace87183645c33086f Mon Sep 17 00:00:00 2001 From: genvalen Date: Thu, 13 Jan 2022 16:34:42 -0500 Subject: [PATCH 02/13] verbose: add tests and validation --- sklearn/ensemble/_voting.py | 10 +++++++++- sklearn/ensemble/tests/test_voting.py | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/sklearn/ensemble/_voting.py b/sklearn/ensemble/_voting.py index 1f6d5508a96ff..e37eed11b76b5 100644 --- a/sklearn/ensemble/_voting.py +++ b/sklearn/ensemble/_voting.py @@ -15,6 +15,7 @@ from abc import abstractmethod +import numbers import numpy as np from joblib import Parallel @@ -27,6 +28,7 @@ from ._base import _BaseHeterogeneousEnsemble from ..preprocessing import LabelEncoder from ..utils import Bunch +from ..utils import check_scalar from ..utils.metaestimators import available_if from ..utils.validation import check_is_fitted from ..utils.multiclass import check_classification_targets @@ -62,9 +64,15 @@ def _predict(self, X): @abstractmethod def fit(self, X, y, sample_weight=None): """Get common fit operations.""" - # check scalar parameters names, clfs = self._validate_estimators() + check_scalar( + self.verbose, + name="verbose", + target_type=(numbers.Integral, np.bool_), + min_val=0, + ) + if self.weights is not None and len(self.weights) != len(self.estimators): raise ValueError( "Number of `estimators` and weights must be equal" diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 4bebfaca53709..7d5b869486f23 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -34,6 +34,28 @@ X_r, y_r = datasets.load_diabetes(return_X_y=True) +@pytest.mark.parametrize( + "X, y, voter, learner", + [ + (X, y, VotingClassifier, {"estimators": [("lr", LogisticRegression())]}), + (X_r, y_r, VotingRegressor, {"estimators": [("lr", LinearRegression())]}), + ] +) +@pytest.mark.parametrize( + "params, err_type, err_msg", + [ + ({"verbose": -1}, ValueError, "verbose == -1, must be >= 0"), + ({"verbose": "foo"}, TypeError, "verbose must be an instance of"), + ] +) +def test_voting_estimators_param_validation(X, y, voter, learner, params, err_type, err_msg): + # Test scalar parameters that are invalid + params.update(learner) + est = voter(**params) + with pytest.raises(err_type, match=err_msg): + est.fit(X, y) + + @pytest.mark.parametrize( "params, err_msg", [ From d67a9fcb230a20762bf1e2137e70bb8148dd9ae6 Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 14 Jan 2022 10:17:27 -0500 Subject: [PATCH 03/13] apply black --- sklearn/ensemble/tests/test_voting.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 7d5b869486f23..183d57155dd58 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -39,16 +39,18 @@ [ (X, y, VotingClassifier, {"estimators": [("lr", LogisticRegression())]}), (X_r, y_r, VotingRegressor, {"estimators": [("lr", LinearRegression())]}), - ] + ], ) @pytest.mark.parametrize( "params, err_type, err_msg", [ ({"verbose": -1}, ValueError, "verbose == -1, must be >= 0"), ({"verbose": "foo"}, TypeError, "verbose must be an instance of"), - ] + ], ) -def test_voting_estimators_param_validation(X, y, voter, learner, params, err_type, err_msg): +def test_voting_estimators_param_validation( + X, y, voter, learner, params, err_type, err_msg +): # Test scalar parameters that are invalid params.update(learner) est = voter(**params) From ae4698cceb514ef55dce172698d0f96f9267cbc0 Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 14 Jan 2022 13:30:53 -0500 Subject: [PATCH 04/13] flatten_transform: add tests and validation --- sklearn/ensemble/_voting.py | 16 ++ .../ensemble/tests/test_gradient_boosting.py | 148 ++++++++++++++++++ sklearn/ensemble/tests/test_voting.py | 12 +- 3 files changed, 175 insertions(+), 1 deletion(-) diff --git a/sklearn/ensemble/_voting.py b/sklearn/ensemble/_voting.py index e37eed11b76b5..feace760e5a46 100644 --- a/sklearn/ensemble/_voting.py +++ b/sklearn/ensemble/_voting.py @@ -80,6 +80,16 @@ def fit(self, X, y, sample_weight=None): % (len(self.weights), len(self.estimators)) ) + # if self.n_jobs is not None: + # if isinstance(self.n_jobs, numbers.Integral): + # if self.n_jobs < 0: + # check_scalar( + # self.n_jobs, + # name="n_jobs", + # target_type=numbers.Integral, + # max_val=-1, + # ) + self.estimators_ = Parallel(n_jobs=self.n_jobs)( delayed(_fit_single_estimator)( clone(clf), @@ -321,6 +331,12 @@ def fit(self, X, y, sample_weight=None): "Multilabel and multi-output classification is not supported." ) + check_scalar( + self.flatten_transform, + name="flatten_transform", + target_type=(numbers.Integral, np.bool_), + ) + if self.voting not in ("soft", "hard"): raise ValueError( "Voting must be 'soft' or 'hard'; got (voting=%r)" % self.voting diff --git a/sklearn/ensemble/tests/test_gradient_boosting.py b/sklearn/ensemble/tests/test_gradient_boosting.py index c2045aa35d652..9c4c2963018bd 100644 --- a/sklearn/ensemble/tests/test_gradient_boosting.py +++ b/sklearn/ensemble/tests/test_gradient_boosting.py @@ -81,6 +81,7 @@ def test_classification_toy(loss): @pytest.mark.parametrize( "params, err_msg", [ +<<<<<<< Updated upstream ({"n_estimators": 0}, "n_estimators must be greater than 0"), ({"n_estimators": -1}, "n_estimators must be greater than 0"), ({"learning_rate": 0}, "learning_rate must be greater than 0"), @@ -104,6 +105,153 @@ def test_classification_toy(loss): ({"max_features": 100}, r"max_features must be in \(0, n_features\]"), ({"max_features": -0.1}, r"max_features must be in \(0, n_features\]"), ({"n_iter_no_change": "invalid"}, "n_iter_no_change should either be"), +======= + ({"learning_rate": 0}, ValueError, "learning_rate == 0, must be > 0."), + ({"learning_rate": -1.0}, ValueError, "learning_rate == -1.0, must be > 0."), + ({"n_estimators": 0}, ValueError, "n_estimators == 0, must be >= 1."), + ({"n_estimators": -1}, ValueError, "n_estimators == -1, must be >= 1."), + ( + {"n_estimators": 1.5}, + TypeError, + "n_estimators must be an instance of ,", + ), + ({"loss": "foobar"}, ValueError, "Loss 'foobar' not supported"), + # ({"min_samples_split": 1}, ValueError, "min_samples_split == 1, must be >= 2"), + # ( + # {"min_samples_split": 900}, + # ValueError, + # "min_samples_split == 900, must be <=", + # ), + # ( + # {"min_samples_split": 0.0}, + # ValueError, + # "min_samples_split == 0.0, must be > 0.0", + # ), + # ( + # {"min_samples_split": 1.1}, + # ValueError, + # "min_samples_split == 1.1, must be <= 1.0", + # ), + # ( + # {"min_samples_split": "foo"}, + # TypeError, + # "min_samples_split must be an instance of ", + # ), + # ({"min_samples_leaf": 0}, ValueError, "min_samples_leaf == 0, must be >= 1"), + # ({"min_samples_leaf": 900}, ValueError, "min_samples_leaf == 900, must be <="), + # ({"min_samples_leaf": 0.0}, ValueError, "min_samples_leaf == 0.0, must be > 0"), + # ( + # {"min_samples_leaf": 0.6}, + # ValueError, + # "min_samples_leaf == 0.6, must be <= 0.5", + # ), + # ( + # {"min_samples_leaf": "foo"}, + # TypeError, + # "min_samples_leaf must be an instance of ", + # ), + # ( + # {"min_weight_fraction_leaf": -1}, + # ValueError, + # "min_weight_fraction_leaf == -1, must be >= 0.0", + # ), + # ( + # {"min_weight_fraction_leaf": 0.6}, + # ValueError, + # "min_weight_fraction_leaf == 0.6, must be <= 0.5", + # ), + # ( + # {"min_weight_fraction_leaf": "foo"}, + # TypeError, + # "min_weight_fraction_leaf must be an instance of ", + # ), + # ({"max_depth": -1}, ValueError, "max_depth == -1, must be >= 1"), + # ( + # {"max_depth": 1.1}, + # TypeError, + # "max_depth must be an instance of ", + # ), + # ( + # {"min_impurity_decrease": -1}, + # ValueError, + # "min_impurity_decrease == -1, must be >= 0.0", + # ), + # ( + # {"min_impurity_decrease": "foo"}, + # TypeError, + # "min_impurity_decrease must be an instance of ", + # ), + ({"subsample": 0.0}, ValueError, "subsample == 0.0, must be > 0."), + ({"subsample": 1.1}, ValueError, "subsample == 1.1, must be <= 1."), + ({"subsample": -0.1}, ValueError, "subsample == -0.1, must be > 0."), + ( + {"subsample": "1"}, + TypeError, + "subsample must be an instance of ,", + ), + + ({"init": {}}, ValueError, "The init parameter must be an estimator or 'zero'"), + ({"max_features": 0}, ValueError, "max_features == 0, must be >= 1"), + ({"max_features": 1000}, ValueError, "max_features == 1000, must be <="), + ({"max_features": 0.0}, ValueError, "max_features == 0.0, must be > 0.0"), + ({"max_features": 1.1}, ValueError, "max_features == 1.1, must be <= 1.0"), + ({"max_features": "foobar"}, ValueError, "Invalid value for max_features."), + # ({"ccp_alpha": -1.0}, ValueError, "ccp_alpha == -1.0, must be >= 0.0"), + # ( + # {"ccp_alpha": "foo"}, + # TypeError, + # "ccp_alpha must be an instance of ", + # ), + ({"verbose": -1}, ValueError, "verbose == -1, must be >= 0"), + ( + {"verbose": "foo"}, + TypeError, + "verbose must be an instance of", + ), + # ({"max_leaf_nodes": 0}, ValueError, "max_leaf_nodes == 0, must be >= 2"), + # ( + # {"max_leaf_nodes": 1.5}, + # TypeError, + # "max_leaf_nodes must be an instance of ", + # ), + ({"warm_start": "foo"}, TypeError, "warm_start must be an instance of"), + ( + {"validation_fraction": 0.0}, + ValueError, + "validation_fraction == 0.0, must be > 0.0", + ), + ( + {"validation_fraction": 1.0}, + ValueError, + "validation_fraction == 1.0, must be < 1.0", + ), + ( + {"validation_fraction": "foo"}, + TypeError, + "validation_fraction must be an instance of ", + ), + + ({"n_iter_no_change": -1}, ValueError, "n_iter_no_change == -1, must be >= 1"), + ({"n_iter_no_change": 0}, ValueError, "n_iter_no_change == 0, must be >= 1"), + ( + {"n_iter_no_change": 1.5}, + TypeError, + "n_iter_no_change must be an instance of ,", + ), + ( + {"n_iter_no_change": "invalid"}, + TypeError, + "n_iter_no_change must be an instance of ,", + ), + ({"tol": 0.0}, ValueError, "tol == 0.0, must be > 0.0"), + ( + {"tol": "foo"}, + TypeError, + "tol must be an instance of ,", + ), + + +>>>>>>> Stashed changes ], # Avoid long error messages in test names: # https://github.com/scikit-learn/scikit-learn/issues/21362 diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 183d57155dd58..8321a958811ee 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -34,6 +34,16 @@ X_r, y_r = datasets.load_diabetes(return_X_y=True) +def test_error(): + # Test that proper excetions are raise given invalid input + voter = VotingClassifier( + estimators=[("lr", LogisticRegression())], flatten_transform="foo" + ) + err_msg = "flatten_transform must be an instance of" + with pytest.raises(TypeError, match=err_msg): + voter.fit(X_r, y_r) + + @pytest.mark.parametrize( "X, y, voter, learner", [ @@ -51,7 +61,7 @@ def test_voting_estimators_param_validation( X, y, voter, learner, params, err_type, err_msg ): - # Test scalar parameters that are invalid + # Test that proper excetions are raise given invalid input params.update(learner) est = voter(**params) with pytest.raises(err_type, match=err_msg): From b71dd11ebebe95dedf8f6476cc1a317c48addcb5 Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 14 Jan 2022 13:32:37 -0500 Subject: [PATCH 05/13] update comments --- sklearn/ensemble/tests/test_voting.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 8321a958811ee..9498bb8788210 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -35,7 +35,7 @@ def test_error(): - # Test that proper excetions are raise given invalid input + # Test that invalid input raises the proper exception voter = VotingClassifier( estimators=[("lr", LogisticRegression())], flatten_transform="foo" ) @@ -61,7 +61,7 @@ def test_error(): def test_voting_estimators_param_validation( X, y, voter, learner, params, err_type, err_msg ): - # Test that proper excetions are raise given invalid input + # Test that invalid imput raises the proper exception params.update(learner) est = voter(**params) with pytest.raises(err_type, match=err_msg): From 199083e59e20a348e0adad294281bf3bb7df206c Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 14 Jan 2022 13:51:56 -0500 Subject: [PATCH 06/13] n_jobs: add placeholder tests and validation (range needs to be confirmed) --- sklearn/ensemble/_voting.py | 20 +++++++++++--------- sklearn/ensemble/tests/test_voting.py | 10 ++++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/sklearn/ensemble/_voting.py b/sklearn/ensemble/_voting.py index feace760e5a46..9a851ffca0df8 100644 --- a/sklearn/ensemble/_voting.py +++ b/sklearn/ensemble/_voting.py @@ -80,15 +80,17 @@ def fit(self, X, y, sample_weight=None): % (len(self.weights), len(self.estimators)) ) - # if self.n_jobs is not None: - # if isinstance(self.n_jobs, numbers.Integral): - # if self.n_jobs < 0: - # check_scalar( - # self.n_jobs, - # name="n_jobs", - # target_type=numbers.Integral, - # max_val=-1, - # ) + if self.n_jobs is not None: + if not isinstance(self.n_jobs, numbers.Integral): + raise TypeError( + "n_jobs must be an instance of , not" + f" {type(self.n_jobs)}" + ) + else: + if self.n_jobs == 0: + raise ValueError( + "n_jobs == 0, must be less than 0 or greater than 0." + ) self.estimators_ = Parallel(n_jobs=self.n_jobs)( delayed(_fit_single_estimator)( diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 9498bb8788210..09f9df709c655 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -54,6 +54,16 @@ def test_error(): @pytest.mark.parametrize( "params, err_type, err_msg", [ + ( + {"n_jobs": 0}, + ValueError, + "n_jobs == 0, must be less than 0 or greater than 0.", + ), + ( + {"n_jobs": "foo"}, + TypeError, + "n_jobs must be an instance of ,", + ), ({"verbose": -1}, ValueError, "verbose == -1, must be >= 0"), ({"verbose": "foo"}, TypeError, "verbose must be an instance of"), ], From 0f61422f425e304f5ea1eccd321ef27c2792ab49 Mon Sep 17 00:00:00 2001 From: genvalen Date: Fri, 14 Jan 2022 14:06:13 -0500 Subject: [PATCH 07/13] Revert "flatten_transform: add tests and validation" This reverts commit ae4698cceb514ef55dce172698d0f96f9267cbc0. --- .../ensemble/tests/test_gradient_boosting.py | 148 ------------------ 1 file changed, 148 deletions(-) diff --git a/sklearn/ensemble/tests/test_gradient_boosting.py b/sklearn/ensemble/tests/test_gradient_boosting.py index 9c4c2963018bd..c2045aa35d652 100644 --- a/sklearn/ensemble/tests/test_gradient_boosting.py +++ b/sklearn/ensemble/tests/test_gradient_boosting.py @@ -81,7 +81,6 @@ def test_classification_toy(loss): @pytest.mark.parametrize( "params, err_msg", [ -<<<<<<< Updated upstream ({"n_estimators": 0}, "n_estimators must be greater than 0"), ({"n_estimators": -1}, "n_estimators must be greater than 0"), ({"learning_rate": 0}, "learning_rate must be greater than 0"), @@ -105,153 +104,6 @@ def test_classification_toy(loss): ({"max_features": 100}, r"max_features must be in \(0, n_features\]"), ({"max_features": -0.1}, r"max_features must be in \(0, n_features\]"), ({"n_iter_no_change": "invalid"}, "n_iter_no_change should either be"), -======= - ({"learning_rate": 0}, ValueError, "learning_rate == 0, must be > 0."), - ({"learning_rate": -1.0}, ValueError, "learning_rate == -1.0, must be > 0."), - ({"n_estimators": 0}, ValueError, "n_estimators == 0, must be >= 1."), - ({"n_estimators": -1}, ValueError, "n_estimators == -1, must be >= 1."), - ( - {"n_estimators": 1.5}, - TypeError, - "n_estimators must be an instance of ,", - ), - ({"loss": "foobar"}, ValueError, "Loss 'foobar' not supported"), - # ({"min_samples_split": 1}, ValueError, "min_samples_split == 1, must be >= 2"), - # ( - # {"min_samples_split": 900}, - # ValueError, - # "min_samples_split == 900, must be <=", - # ), - # ( - # {"min_samples_split": 0.0}, - # ValueError, - # "min_samples_split == 0.0, must be > 0.0", - # ), - # ( - # {"min_samples_split": 1.1}, - # ValueError, - # "min_samples_split == 1.1, must be <= 1.0", - # ), - # ( - # {"min_samples_split": "foo"}, - # TypeError, - # "min_samples_split must be an instance of ", - # ), - # ({"min_samples_leaf": 0}, ValueError, "min_samples_leaf == 0, must be >= 1"), - # ({"min_samples_leaf": 900}, ValueError, "min_samples_leaf == 900, must be <="), - # ({"min_samples_leaf": 0.0}, ValueError, "min_samples_leaf == 0.0, must be > 0"), - # ( - # {"min_samples_leaf": 0.6}, - # ValueError, - # "min_samples_leaf == 0.6, must be <= 0.5", - # ), - # ( - # {"min_samples_leaf": "foo"}, - # TypeError, - # "min_samples_leaf must be an instance of ", - # ), - # ( - # {"min_weight_fraction_leaf": -1}, - # ValueError, - # "min_weight_fraction_leaf == -1, must be >= 0.0", - # ), - # ( - # {"min_weight_fraction_leaf": 0.6}, - # ValueError, - # "min_weight_fraction_leaf == 0.6, must be <= 0.5", - # ), - # ( - # {"min_weight_fraction_leaf": "foo"}, - # TypeError, - # "min_weight_fraction_leaf must be an instance of ", - # ), - # ({"max_depth": -1}, ValueError, "max_depth == -1, must be >= 1"), - # ( - # {"max_depth": 1.1}, - # TypeError, - # "max_depth must be an instance of ", - # ), - # ( - # {"min_impurity_decrease": -1}, - # ValueError, - # "min_impurity_decrease == -1, must be >= 0.0", - # ), - # ( - # {"min_impurity_decrease": "foo"}, - # TypeError, - # "min_impurity_decrease must be an instance of ", - # ), - ({"subsample": 0.0}, ValueError, "subsample == 0.0, must be > 0."), - ({"subsample": 1.1}, ValueError, "subsample == 1.1, must be <= 1."), - ({"subsample": -0.1}, ValueError, "subsample == -0.1, must be > 0."), - ( - {"subsample": "1"}, - TypeError, - "subsample must be an instance of ,", - ), - - ({"init": {}}, ValueError, "The init parameter must be an estimator or 'zero'"), - ({"max_features": 0}, ValueError, "max_features == 0, must be >= 1"), - ({"max_features": 1000}, ValueError, "max_features == 1000, must be <="), - ({"max_features": 0.0}, ValueError, "max_features == 0.0, must be > 0.0"), - ({"max_features": 1.1}, ValueError, "max_features == 1.1, must be <= 1.0"), - ({"max_features": "foobar"}, ValueError, "Invalid value for max_features."), - # ({"ccp_alpha": -1.0}, ValueError, "ccp_alpha == -1.0, must be >= 0.0"), - # ( - # {"ccp_alpha": "foo"}, - # TypeError, - # "ccp_alpha must be an instance of ", - # ), - ({"verbose": -1}, ValueError, "verbose == -1, must be >= 0"), - ( - {"verbose": "foo"}, - TypeError, - "verbose must be an instance of", - ), - # ({"max_leaf_nodes": 0}, ValueError, "max_leaf_nodes == 0, must be >= 2"), - # ( - # {"max_leaf_nodes": 1.5}, - # TypeError, - # "max_leaf_nodes must be an instance of ", - # ), - ({"warm_start": "foo"}, TypeError, "warm_start must be an instance of"), - ( - {"validation_fraction": 0.0}, - ValueError, - "validation_fraction == 0.0, must be > 0.0", - ), - ( - {"validation_fraction": 1.0}, - ValueError, - "validation_fraction == 1.0, must be < 1.0", - ), - ( - {"validation_fraction": "foo"}, - TypeError, - "validation_fraction must be an instance of ", - ), - - ({"n_iter_no_change": -1}, ValueError, "n_iter_no_change == -1, must be >= 1"), - ({"n_iter_no_change": 0}, ValueError, "n_iter_no_change == 0, must be >= 1"), - ( - {"n_iter_no_change": 1.5}, - TypeError, - "n_iter_no_change must be an instance of ,", - ), - ( - {"n_iter_no_change": "invalid"}, - TypeError, - "n_iter_no_change must be an instance of ,", - ), - ({"tol": 0.0}, ValueError, "tol == 0.0, must be > 0.0"), - ( - {"tol": "foo"}, - TypeError, - "tol must be an instance of ,", - ), - - ->>>>>>> Stashed changes ], # Avoid long error messages in test names: # https://github.com/scikit-learn/scikit-learn/issues/21362 From a2408339f9117acfa68a8922846886a6830bf5ad Mon Sep 17 00:00:00 2001 From: genvalen Date: Sun, 16 Jan 2022 11:08:42 -0500 Subject: [PATCH 08/13] update classifier's input data to use iris dataset --- sklearn/ensemble/tests/test_voting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 09f9df709c655..09c55b199180d 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -41,7 +41,7 @@ def test_error(): ) err_msg = "flatten_transform must be an instance of" with pytest.raises(TypeError, match=err_msg): - voter.fit(X_r, y_r) + voter.fit(X, y) @pytest.mark.parametrize( From ec2966e43eed1af725f30be36c633f721e5ea92a Mon Sep 17 00:00:00 2001 From: genvalen Date: Sun, 16 Jan 2022 15:00:23 -0500 Subject: [PATCH 09/13] Update variable names for consistency --- sklearn/ensemble/tests/test_voting.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 09c55b199180d..ccd1ab5acf0c2 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -36,12 +36,12 @@ def test_error(): # Test that invalid input raises the proper exception - voter = VotingClassifier( + ensemble = VotingClassifier( estimators=[("lr", LogisticRegression())], flatten_transform="foo" ) err_msg = "flatten_transform must be an instance of" with pytest.raises(TypeError, match=err_msg): - voter.fit(X, y) + ensemble.fit(X, y) @pytest.mark.parametrize( @@ -73,9 +73,9 @@ def test_voting_estimators_param_validation( ): # Test that invalid imput raises the proper exception params.update(learner) - est = voter(**params) + ensemble = voter(**params) with pytest.raises(err_type, match=err_msg): - est.fit(X, y) + ensemble.fit(X, y) @pytest.mark.parametrize( From b317f6844b17f55aef4e749f96be5149b3f4452a Mon Sep 17 00:00:00 2001 From: genvalen Date: Sun, 16 Jan 2022 15:35:58 -0500 Subject: [PATCH 10/13] small edit: update string formatting to fstring --- sklearn/ensemble/_voting.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sklearn/ensemble/_voting.py b/sklearn/ensemble/_voting.py index 9a851ffca0df8..6d70ba85f647f 100644 --- a/sklearn/ensemble/_voting.py +++ b/sklearn/ensemble/_voting.py @@ -48,7 +48,7 @@ class _BaseVoting(TransformerMixin, _BaseHeterogeneousEnsemble): def _log_message(self, name, idx, total): if not self.verbose: return None - return "(%d of %d) Processing %s" % (idx, total, name) + return f"({idx} of {total}) Processing {name}" @property def _weights_not_none(self): @@ -75,9 +75,8 @@ def fit(self, X, y, sample_weight=None): if self.weights is not None and len(self.weights) != len(self.estimators): raise ValueError( - "Number of `estimators` and weights must be equal" - "; got %d weights, %d estimators" - % (len(self.weights), len(self.estimators)) + "Number of `estimators` and weights must be equal; got" + f" {len(self.weights)} weights, {len(self.estimators)} estimators" ) if self.n_jobs is not None: @@ -341,7 +340,7 @@ def fit(self, X, y, sample_weight=None): if self.voting not in ("soft", "hard"): raise ValueError( - "Voting must be 'soft' or 'hard'; got (voting=%r)" % self.voting + f"Voting must be 'soft' or 'hard'; got (voting={self.voting!r})" ) self.le_ = LabelEncoder().fit(y) From 93fd5df2ddd90c45826703ed2d03c28a09a86841 Mon Sep 17 00:00:00 2001 From: genvalen Date: Mon, 24 Jan 2022 13:58:35 -0500 Subject: [PATCH 11/13] n_jobs: remove tets and validation --- sklearn/ensemble/_voting.py | 12 ------------ sklearn/ensemble/tests/test_voting.py | 10 ---------- 2 files changed, 22 deletions(-) diff --git a/sklearn/ensemble/_voting.py b/sklearn/ensemble/_voting.py index 6d70ba85f647f..0bfc458f3eaff 100644 --- a/sklearn/ensemble/_voting.py +++ b/sklearn/ensemble/_voting.py @@ -79,18 +79,6 @@ def fit(self, X, y, sample_weight=None): f" {len(self.weights)} weights, {len(self.estimators)} estimators" ) - if self.n_jobs is not None: - if not isinstance(self.n_jobs, numbers.Integral): - raise TypeError( - "n_jobs must be an instance of , not" - f" {type(self.n_jobs)}" - ) - else: - if self.n_jobs == 0: - raise ValueError( - "n_jobs == 0, must be less than 0 or greater than 0." - ) - self.estimators_ = Parallel(n_jobs=self.n_jobs)( delayed(_fit_single_estimator)( clone(clf), diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index ccd1ab5acf0c2..3979b1f357c81 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -54,16 +54,6 @@ def test_error(): @pytest.mark.parametrize( "params, err_type, err_msg", [ - ( - {"n_jobs": 0}, - ValueError, - "n_jobs == 0, must be less than 0 or greater than 0.", - ), - ( - {"n_jobs": "foo"}, - TypeError, - "n_jobs must be an instance of ,", - ), ({"verbose": -1}, ValueError, "verbose == -1, must be >= 0"), ({"verbose": "foo"}, TypeError, "verbose must be an instance of"), ], From c755f629e53dcad3efa92529a22c0f2f859dcf4f Mon Sep 17 00:00:00 2001 From: genvalen Date: Mon, 24 Jan 2022 13:59:30 -0500 Subject: [PATCH 12/13] Update sklearn/ensemble/tests/test_voting.py Co-authored-by: Olivier Grisel --- sklearn/ensemble/tests/test_voting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 3979b1f357c81..55be55bcebe57 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -34,7 +34,7 @@ X_r, y_r = datasets.load_diabetes(return_X_y=True) -def test_error(): +def test_invalid_type_for_flatten_transform(): # Test that invalid input raises the proper exception ensemble = VotingClassifier( estimators=[("lr", LogisticRegression())], flatten_transform="foo" From 5a5e1fb7a38752f801a8a125b16652bad4b0c65a Mon Sep 17 00:00:00 2001 From: genvalen Date: Mon, 24 Jan 2022 13:59:35 -0500 Subject: [PATCH 13/13] Update sklearn/ensemble/tests/test_voting.py Co-authored-by: Olivier Grisel --- sklearn/ensemble/tests/test_voting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/ensemble/tests/test_voting.py b/sklearn/ensemble/tests/test_voting.py index 55be55bcebe57..ab11cff022fd2 100644 --- a/sklearn/ensemble/tests/test_voting.py +++ b/sklearn/ensemble/tests/test_voting.py @@ -61,7 +61,7 @@ def test_invalid_type_for_flatten_transform(): def test_voting_estimators_param_validation( X, y, voter, learner, params, err_type, err_msg ): - # Test that invalid imput raises the proper exception + # Test that invalid input raises the proper exception params.update(learner) ensemble = voter(**params) with pytest.raises(err_type, match=err_msg):