From 5ccfd34528257ab297f4b962b9c422bb57c18433 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Fri, 14 Jan 2022 20:56:09 -0500 Subject: [PATCH 01/10] ENH Better error message for check_scalar --- doc/whats_new/v1.1.rst | 3 +++ sklearn/cluster/tests/test_birch.py | 6 ++---- sklearn/cluster/tests/test_dbscan.py | 9 +++------ sklearn/cluster/tests/test_spectral.py | 6 ++---- sklearn/decomposition/tests/test_pca.py | 2 +- .../ensemble/tests/test_weight_boosting.py | 3 +-- sklearn/feature_extraction/tests/test_text.py | 3 +-- sklearn/linear_model/tests/test_ridge.py | 10 ++++------ .../tests/test_discretization.py | 5 +---- sklearn/utils/tests/test_validation.py | 13 ++++++++++--- sklearn/utils/validation.py | 19 ++++++++++++++++++- 11 files changed, 46 insertions(+), 33 deletions(-) diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst index 5339f4a399490..fddff4aaa520b 100644 --- a/doc/whats_new/v1.1.rst +++ b/doc/whats_new/v1.1.rst @@ -486,6 +486,9 @@ Changelog left corner of the HTML representation to show how the elements are clickable. :pr:`21298` by `Thomas Fan`_. +- |Enhancement| :func:`utils.validation.check_scalar` now has better messages + when displaying the type. :pr:`xxxxx` by `Thomas Fan`_. + Code and Documentation Contributors ----------------------------------- diff --git a/sklearn/cluster/tests/test_birch.py b/sklearn/cluster/tests/test_birch.py index 5d8a3222ef156..d173b05788a5c 100644 --- a/sklearn/cluster/tests/test_birch.py +++ b/sklearn/cluster/tests/test_birch.py @@ -195,16 +195,14 @@ def test_birch_fit_attributes_deprecated(attribute): ( {"branching_factor": 1.5}, TypeError, - "branching_factor must be an instance of , not" - " .", + "branching_factor must be an instance of numbers.Integral, not float.", ), ({"branching_factor": -2}, ValueError, "branching_factor == -2, must be > 1."), ({"n_clusters": 0}, ValueError, "n_clusters == 0, must be >= 1."), ( {"n_clusters": 2.5}, TypeError, - "n_clusters must be an instance of , not .", + "n_clusters must be an instance of numbers.Integral, not float.", ), ( {"n_clusters": "whatever"}, diff --git a/sklearn/cluster/tests/test_dbscan.py b/sklearn/cluster/tests/test_dbscan.py index 08ca937d3e25f..b67a8bfa00965 100644 --- a/sklearn/cluster/tests/test_dbscan.py +++ b/sklearn/cluster/tests/test_dbscan.py @@ -436,24 +436,21 @@ def test_dbscan_precomputed_metric_with_initial_rows_zero(): ( {"min_samples": 1.5}, TypeError, - "min_samples must be an instance of , not .", + "min_samples must be an instance of numbers.Integral, not float.", ), ({"min_samples": -2}, ValueError, "min_samples == -2, must be >= 1."), ({"leaf_size": 0}, ValueError, "leaf_size == 0, must be >= 1."), ( {"leaf_size": 2.5}, TypeError, - "leaf_size must be an instance of , not .", + "leaf_size must be an instance of numbers.Integral, not float.", ), ({"leaf_size": -3}, ValueError, "leaf_size == -3, must be >= 1."), ({"p": -2}, ValueError, "p == -2, must be >= 0.0."), ( {"n_jobs": 2.5}, TypeError, - "n_jobs must be an instance of , not .", + "n_jobs must be an instance of numbers.Integral, not float.", ), ], ) diff --git a/sklearn/cluster/tests/test_spectral.py b/sklearn/cluster/tests/test_spectral.py index 29785c0869fed..5fe7269c27184 100644 --- a/sklearn/cluster/tests/test_spectral.py +++ b/sklearn/cluster/tests/test_spectral.py @@ -121,8 +121,7 @@ def test_spectral_unknown_assign_labels(): X, {"n_clusters": 1.5}, TypeError, - "n_clusters must be an instance of ," - " not ", + "n_clusters must be an instance of numbers.Integral, not float", ), (X, {"n_init": -1}, ValueError, "n_init == -1, must be >= 1"), (X, {"n_init": 0}, ValueError, "n_init == 0, must be >= 1"), @@ -130,8 +129,7 @@ def test_spectral_unknown_assign_labels(): X, {"n_init": 1.5}, TypeError, - "n_init must be an instance of ," - " not ", + "n_init must be an instance of numbers.Integral, not float", ), (X, {"gamma": -1}, ValueError, "gamma == -1, must be >= 1"), (X, {"gamma": 0}, ValueError, "gamma == 0, must be >= 1"), diff --git a/sklearn/decomposition/tests/test_pca.py b/sklearn/decomposition/tests/test_pca.py index 145a76cb9551a..4595853c1410c 100644 --- a/sklearn/decomposition/tests/test_pca.py +++ b/sklearn/decomposition/tests/test_pca.py @@ -696,7 +696,7 @@ def test_pca_randomized_svd_n_oversamples(): ( {"n_oversamples": 1.5}, TypeError, - "n_oversamples must be an instance of ", + "n_oversamples must be an instance of numbers.Integral", ), ], ) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index 5027dbd02c859..8980943b64bde 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -564,8 +564,7 @@ def test_adaboostregressor_sample_weight(): ( {"n_estimators": 1.5}, TypeError, - "n_estimators must be an instance of ," - " not ", + "n_estimators must be an instance of numbers.Integral, not float", ), ({"learning_rate": -1}, ValueError, "learning_rate == -1, must be > 0."), ({"learning_rate": 0}, ValueError, "learning_rate == 0, must be > 0."), diff --git a/sklearn/feature_extraction/tests/test_text.py b/sklearn/feature_extraction/tests/test_text.py index f903ccc89d4dc..a0b33cdb93660 100644 --- a/sklearn/feature_extraction/tests/test_text.py +++ b/sklearn/feature_extraction/tests/test_text.py @@ -869,8 +869,7 @@ def test_vectorizer_min_df(): ( {"max_features": 3.5}, TypeError, - "max_features must be an instance of , not ", + "max_features must be an instance of numbers.Integral, not float", ), ), ) diff --git a/sklearn/linear_model/tests/test_ridge.py b/sklearn/linear_model/tests/test_ridge.py index 975d16df06f12..a468ea35986ba 100644 --- a/sklearn/linear_model/tests/test_ridge.py +++ b/sklearn/linear_model/tests/test_ridge.py @@ -342,20 +342,19 @@ def test_ridge_individual_penalties(): ( {"alpha": "1"}, TypeError, - "alpha must be an instance of , not ", + "alpha must be an instance of numbers.Real, not str", ), ({"max_iter": 0}, ValueError, "max_iter == 0, must be >= 1."), ( {"max_iter": "1"}, TypeError, - "max_iter must be an instance of , not ", + "max_iter must be an instance of numbers.Integral, not str", ), ({"tol": -1.0}, ValueError, "tol == -1.0, must be >= 0."), ( {"tol": "1"}, TypeError, - "tol must be an instance of , not ", + "tol must be an instance of numbers.Real, not str", ), ], ) @@ -1283,8 +1282,7 @@ def test_ridgecv_int_alphas(): ( {"alphas": (1, 1.0, "1")}, TypeError, - r"alphas\[2\] must be an instance of , not ", + r"alphas\[2\] must be an instance of numbers.Real, not str", ), ], ) diff --git a/sklearn/preprocessing/tests/test_discretization.py b/sklearn/preprocessing/tests/test_discretization.py index fa8240893f7c3..1c79685f75ae7 100644 --- a/sklearn/preprocessing/tests/test_discretization.py +++ b/sklearn/preprocessing/tests/test_discretization.py @@ -395,10 +395,7 @@ def test_kbinsdiscretizer_subsample_invalid_type(): n_bins=10, encode="ordinal", strategy="quantile", subsample="full" ) - msg = ( - "subsample must be an instance of , not " - "." - ) + msg = "subsample must be an instance of numbers.Integral, not str." with pytest.raises(TypeError, match=msg): kbd.fit(X) diff --git a/sklearn/utils/tests/test_validation.py b/sklearn/utils/tests/test_validation.py index 18f88373b02f3..ee44913a92f83 100644 --- a/sklearn/utils/tests/test_validation.py +++ b/sklearn/utils/tests/test_validation.py @@ -1106,9 +1106,16 @@ def test_check_scalar_valid(x): 2, 4, "neither", - TypeError( - "test_name1 must be an instance of , not ." - ), + TypeError("test_name1 must be an instance of float, not int."), + ), + ( + 1, + "test_name1", + (float, bool), + 2, + 4, + "neither", + TypeError("test_name1 must be an instance of {float, bool}, not int."), ), ( 1, diff --git a/sklearn/utils/validation.py b/sklearn/utils/validation.py index 5936415c776b8..abed47dec76d4 100644 --- a/sklearn/utils/validation.py +++ b/sklearn/utils/validation.py @@ -1409,7 +1409,24 @@ def check_scalar( """ if not isinstance(x, target_type): - raise TypeError(f"{name} must be an instance of {target_type}, not {type(x)}.") + + def type_name(t): + module = t.__module__ + qualname = t.__qualname__ + if module == "builtins": + return qualname + return f"{module}.{qualname}" + + if isinstance(target_type, tuple): + types_str = ", ".join(type_name(t) for t in target_type) + target_type_str = f"{{{types_str}}}" + else: + target_type_str = type_name(target_type) + + raise TypeError( + f"{name} must be an instance of {target_type_str}, not" + f" {type(x).__qualname__}." + ) expected_include_boundaries = ("left", "right", "both", "neither") if include_boundaries not in expected_include_boundaries: From 2741f3257004b3aa39c3aae64b6ecbbf9f23ba62 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Fri, 14 Jan 2022 20:58:16 -0500 Subject: [PATCH 02/10] DOC Adds PR number --- doc/whats_new/v1.1.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst index fddff4aaa520b..c23624c534aad 100644 --- a/doc/whats_new/v1.1.rst +++ b/doc/whats_new/v1.1.rst @@ -487,7 +487,7 @@ Changelog clickable. :pr:`21298` by `Thomas Fan`_. - |Enhancement| :func:`utils.validation.check_scalar` now has better messages - when displaying the type. :pr:`xxxxx` by `Thomas Fan`_. + when displaying the type. :pr:`22218` by `Thomas Fan`_. Code and Documentation Contributors ----------------------------------- From f4dac8bb54b652cf0f6c6fc1c91b93b0504a8862 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Mon, 24 Jan 2022 12:37:01 -0500 Subject: [PATCH 03/10] ENH Adjust names for numbers module --- sklearn/utils/validation.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sklearn/utils/validation.py b/sklearn/utils/validation.py index 9644744664acb..a1a3fc4121d3e 100644 --- a/sklearn/utils/validation.py +++ b/sklearn/utils/validation.py @@ -1411,10 +1411,16 @@ def check_scalar( if not isinstance(x, target_type): def type_name(t): + """Convert type into humman readable string.""" module = t.__module__ qualname = t.__qualname__ if module == "builtins": return qualname + elif module == "numbers": + if qualname == "Real": + return "float" + elif qualname == "Integral": + return "int" return f"{module}.{qualname}" if isinstance(target_type, tuple): From f18829a8dbdbc83a21d441d2b5c05f5dd6dde223 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Mon, 24 Jan 2022 12:53:44 -0500 Subject: [PATCH 04/10] CLN Update tests to use float/int --- sklearn/cluster/tests/test_birch.py | 4 ++-- sklearn/cluster/tests/test_dbscan.py | 6 +++--- sklearn/cluster/tests/test_spectral.py | 4 ++-- sklearn/decomposition/tests/test_pca.py | 2 +- .../ensemble/tests/test_weight_boosting.py | 2 +- sklearn/feature_extraction/tests/test_text.py | 2 +- sklearn/linear_model/_glm/tests/test_glm.py | 21 +++++++------------ .../tests/test_coordinate_descent.py | 7 +++---- sklearn/linear_model/tests/test_ridge.py | 8 +++---- .../tests/test_discretization.py | 2 +- 10 files changed, 26 insertions(+), 32 deletions(-) diff --git a/sklearn/cluster/tests/test_birch.py b/sklearn/cluster/tests/test_birch.py index d173b05788a5c..78484ae39b488 100644 --- a/sklearn/cluster/tests/test_birch.py +++ b/sklearn/cluster/tests/test_birch.py @@ -195,14 +195,14 @@ def test_birch_fit_attributes_deprecated(attribute): ( {"branching_factor": 1.5}, TypeError, - "branching_factor must be an instance of numbers.Integral, not float.", + "branching_factor must be an instance of int, not float.", ), ({"branching_factor": -2}, ValueError, "branching_factor == -2, must be > 1."), ({"n_clusters": 0}, ValueError, "n_clusters == 0, must be >= 1."), ( {"n_clusters": 2.5}, TypeError, - "n_clusters must be an instance of numbers.Integral, not float.", + "n_clusters must be an instance of int, not float.", ), ( {"n_clusters": "whatever"}, diff --git a/sklearn/cluster/tests/test_dbscan.py b/sklearn/cluster/tests/test_dbscan.py index b67a8bfa00965..b3b58b7a79b4b 100644 --- a/sklearn/cluster/tests/test_dbscan.py +++ b/sklearn/cluster/tests/test_dbscan.py @@ -436,21 +436,21 @@ def test_dbscan_precomputed_metric_with_initial_rows_zero(): ( {"min_samples": 1.5}, TypeError, - "min_samples must be an instance of numbers.Integral, not float.", + "min_samples must be an instance of int, not float.", ), ({"min_samples": -2}, ValueError, "min_samples == -2, must be >= 1."), ({"leaf_size": 0}, ValueError, "leaf_size == 0, must be >= 1."), ( {"leaf_size": 2.5}, TypeError, - "leaf_size must be an instance of numbers.Integral, not float.", + "leaf_size must be an instance of int, not float.", ), ({"leaf_size": -3}, ValueError, "leaf_size == -3, must be >= 1."), ({"p": -2}, ValueError, "p == -2, must be >= 0.0."), ( {"n_jobs": 2.5}, TypeError, - "n_jobs must be an instance of numbers.Integral, not float.", + "n_jobs must be an instance of int, not float.", ), ], ) diff --git a/sklearn/cluster/tests/test_spectral.py b/sklearn/cluster/tests/test_spectral.py index 5fe7269c27184..43be5497763fa 100644 --- a/sklearn/cluster/tests/test_spectral.py +++ b/sklearn/cluster/tests/test_spectral.py @@ -121,7 +121,7 @@ def test_spectral_unknown_assign_labels(): X, {"n_clusters": 1.5}, TypeError, - "n_clusters must be an instance of numbers.Integral, not float", + "n_clusters must be an instance of int, not float", ), (X, {"n_init": -1}, ValueError, "n_init == -1, must be >= 1"), (X, {"n_init": 0}, ValueError, "n_init == 0, must be >= 1"), @@ -129,7 +129,7 @@ def test_spectral_unknown_assign_labels(): X, {"n_init": 1.5}, TypeError, - "n_init must be an instance of numbers.Integral, not float", + "n_init must be an instance of int, not float", ), (X, {"gamma": -1}, ValueError, "gamma == -1, must be >= 1"), (X, {"gamma": 0}, ValueError, "gamma == 0, must be >= 1"), diff --git a/sklearn/decomposition/tests/test_pca.py b/sklearn/decomposition/tests/test_pca.py index 4595853c1410c..e9036d8a6bed8 100644 --- a/sklearn/decomposition/tests/test_pca.py +++ b/sklearn/decomposition/tests/test_pca.py @@ -696,7 +696,7 @@ def test_pca_randomized_svd_n_oversamples(): ( {"n_oversamples": 1.5}, TypeError, - "n_oversamples must be an instance of numbers.Integral", + "n_oversamples must be an instance of int", ), ], ) diff --git a/sklearn/ensemble/tests/test_weight_boosting.py b/sklearn/ensemble/tests/test_weight_boosting.py index 8980943b64bde..0348641d39453 100755 --- a/sklearn/ensemble/tests/test_weight_boosting.py +++ b/sklearn/ensemble/tests/test_weight_boosting.py @@ -564,7 +564,7 @@ def test_adaboostregressor_sample_weight(): ( {"n_estimators": 1.5}, TypeError, - "n_estimators must be an instance of numbers.Integral, not float", + "n_estimators must be an instance of int, not float", ), ({"learning_rate": -1}, ValueError, "learning_rate == -1, must be > 0."), ({"learning_rate": 0}, ValueError, "learning_rate == 0, must be > 0."), diff --git a/sklearn/feature_extraction/tests/test_text.py b/sklearn/feature_extraction/tests/test_text.py index a0b33cdb93660..bb33adba9730f 100644 --- a/sklearn/feature_extraction/tests/test_text.py +++ b/sklearn/feature_extraction/tests/test_text.py @@ -869,7 +869,7 @@ def test_vectorizer_min_df(): ( {"max_features": 3.5}, TypeError, - "max_features must be an instance of numbers.Integral, not float", + "max_features must be an instance of int, not float", ), ), ) diff --git a/sklearn/linear_model/_glm/tests/test_glm.py b/sklearn/linear_model/_glm/tests/test_glm.py index b90e273cbd246..87fe2b51f4d28 100644 --- a/sklearn/linear_model/_glm/tests/test_glm.py +++ b/sklearn/linear_model/_glm/tests/test_glm.py @@ -142,26 +142,23 @@ def test_glm_solver_argument(solver): ( {"max_iter": "not a number"}, TypeError, - "max_iter must be an instance of , not ", + "max_iter must be an instance of int, not str", ), ( {"max_iter": [1]}, TypeError, - "max_iter must be an instance of ," - " not ", + "max_iter must be an instance of int, not list", ), ( {"max_iter": 5.5}, TypeError, - "max_iter must be an instance of ," - " not ", + "max_iter must be an instance of int, not float", ), ({"alpha": -1}, ValueError, "alpha == -1, must be >= 0.0"), ( {"alpha": "1"}, TypeError, - "alpha must be an instance of , not ", + "alpha must be an instance of float, not str", ), ({"tol": -1.0}, ValueError, "tol == -1.0, must be > 0."), ({"tol": 0.0}, ValueError, "tol == 0.0, must be > 0.0"), @@ -169,25 +166,23 @@ def test_glm_solver_argument(solver): ( {"tol": "1"}, TypeError, - "tol must be an instance of , not ", + "tol must be an instance of float, not str", ), ( {"tol": [1e-3]}, TypeError, - "tol must be an instance of , not ", + "tol must be an instance of float, not list", ), ({"verbose": -1}, ValueError, "verbose == -1, must be >= 0."), ( {"verbose": "1"}, TypeError, - "verbose must be an instance of , not ", + "verbose must be an instance of int, not str", ), ( {"verbose": 1.0}, TypeError, - "verbose must be an instance of , not ", + "verbose must be an instance of int, not float", ), ], ) diff --git a/sklearn/linear_model/tests/test_coordinate_descent.py b/sklearn/linear_model/tests/test_coordinate_descent.py index 53ff69267dcc9..a03557c24bb90 100644 --- a/sklearn/linear_model/tests/test_coordinate_descent.py +++ b/sklearn/linear_model/tests/test_coordinate_descent.py @@ -115,20 +115,19 @@ def test_assure_warning_when_normalize(CoordinateDescentModel, normalize, n_warn ( {"l1_ratio": "1"}, TypeError, - "l1_ratio must be an instance of , not ", + "l1_ratio must be an instance of float, not str", ), ({"tol": -1.0}, ValueError, "tol == -1.0, must be >= 0."), ( {"tol": "1"}, TypeError, - "tol must be an instance of , not ", + "tol must be an instance of float, not str", ), ({"max_iter": 0}, ValueError, "max_iter == 0, must be >= 1."), ( {"max_iter": "1"}, TypeError, - "max_iter must be an instance of , not ", + "max_iter must be an instance of int, not str", ), ], ) diff --git a/sklearn/linear_model/tests/test_ridge.py b/sklearn/linear_model/tests/test_ridge.py index a468ea35986ba..aa2cac29c60de 100644 --- a/sklearn/linear_model/tests/test_ridge.py +++ b/sklearn/linear_model/tests/test_ridge.py @@ -342,19 +342,19 @@ def test_ridge_individual_penalties(): ( {"alpha": "1"}, TypeError, - "alpha must be an instance of numbers.Real, not str", + "alpha must be an instance of float, not str", ), ({"max_iter": 0}, ValueError, "max_iter == 0, must be >= 1."), ( {"max_iter": "1"}, TypeError, - "max_iter must be an instance of numbers.Integral, not str", + "max_iter must be an instance of int, not str", ), ({"tol": -1.0}, ValueError, "tol == -1.0, must be >= 0."), ( {"tol": "1"}, TypeError, - "tol must be an instance of numbers.Real, not str", + "tol must be an instance of float, not str", ), ], ) @@ -1282,7 +1282,7 @@ def test_ridgecv_int_alphas(): ( {"alphas": (1, 1.0, "1")}, TypeError, - r"alphas\[2\] must be an instance of numbers.Real, not str", + r"alphas\[2\] must be an instance of float, not str", ), ], ) diff --git a/sklearn/preprocessing/tests/test_discretization.py b/sklearn/preprocessing/tests/test_discretization.py index 1c79685f75ae7..51e854eba40b1 100644 --- a/sklearn/preprocessing/tests/test_discretization.py +++ b/sklearn/preprocessing/tests/test_discretization.py @@ -395,7 +395,7 @@ def test_kbinsdiscretizer_subsample_invalid_type(): n_bins=10, encode="ordinal", strategy="quantile", subsample="full" ) - msg = "subsample must be an instance of numbers.Integral, not str." + msg = "subsample must be an instance of int, not str." with pytest.raises(TypeError, match=msg): kbd.fit(X) From 576db0b787dcd92a924c0753c5acf9dad8b685ad Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Thu, 27 Jan 2022 12:38:41 -0500 Subject: [PATCH 05/10] TST Adjust tests for new type names --- sklearn/tree/tests/test_tree.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sklearn/tree/tests/test_tree.py b/sklearn/tree/tests/test_tree.py index bb8f1509e2fee..a96aca5e8f10e 100644 --- a/sklearn/tree/tests/test_tree.py +++ b/sklearn/tree/tests/test_tree.py @@ -621,14 +621,14 @@ def test_error(): ( {"max_depth": 1.1}, TypeError, - "max_depth must be an instance of ", + "max_depth must be an instance of int", ), ({"min_samples_leaf": 0}, ValueError, "min_samples_leaf == 0, must be >= 1"), ({"min_samples_leaf": 0.0}, ValueError, "min_samples_leaf == 0.0, must be > 0"), ( {"min_samples_leaf": "foo"}, TypeError, - "min_samples_leaf must be an instance of ", + "min_samples_leaf must be an instance of float", ), ({"min_samples_split": 1}, ValueError, "min_samples_split == 1, must be >= 2"), ( @@ -644,7 +644,7 @@ def test_error(): ( {"min_samples_split": "foo"}, TypeError, - "min_samples_split must be an instance of ", + "min_samples_split must be an instance of float", ), ( {"min_weight_fraction_leaf": -1}, @@ -659,7 +659,7 @@ def test_error(): ( {"min_weight_fraction_leaf": "foo"}, TypeError, - "min_weight_fraction_leaf must be an instance of ", + "min_weight_fraction_leaf must be an instance of float", ), ({"max_features": 0}, ValueError, "max_features == 0, must be >= 1"), ({"max_features": 1000}, ValueError, "max_features == 1000, must be <="), @@ -670,7 +670,7 @@ def test_error(): ( {"max_leaf_nodes": 1.5}, TypeError, - "max_leaf_nodes must be an instance of ", + "max_leaf_nodes must be an instance of int", ), ( {"min_impurity_decrease": -1}, @@ -680,13 +680,13 @@ def test_error(): ( {"min_impurity_decrease": "foo"}, TypeError, - "min_impurity_decrease must be an instance of ", + "min_impurity_decrease must be an instance of float", ), ({"ccp_alpha": -1.0}, ValueError, "ccp_alpha == -1.0, must be >= 0.0"), ( {"ccp_alpha": "foo"}, TypeError, - "ccp_alpha must be an instance of ", + "ccp_alpha must be an instance of float", ), ], ) From 47266ad691966a38e0c00a093e817b45f30368ba Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 9 Feb 2022 14:59:03 -0500 Subject: [PATCH 06/10] CLN Clean up main merge --- sklearn/cross_decomposition/tests/test_pls.py | 4 +-- .../ensemble/tests/test_gradient_boosting.py | 26 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/sklearn/cross_decomposition/tests/test_pls.py b/sklearn/cross_decomposition/tests/test_pls.py index 1118e8c33a9c0..7e704ced0f2af 100644 --- a/sklearn/cross_decomposition/tests/test_pls.py +++ b/sklearn/cross_decomposition/tests/test_pls.py @@ -476,7 +476,7 @@ def test_scale_and_stability(Est, X, Y): ( 2.0, TypeError, - "n_components must be an instance of ", + "n_components must be an instance of int", ), ], ) @@ -498,7 +498,7 @@ def test_n_components_bounds(Est, n_components, err_type, err_msg): ( 2.0, TypeError, - "n_components must be an instance of ", + "n_components must be an instance of int", ), ], ) diff --git a/sklearn/ensemble/tests/test_gradient_boosting.py b/sklearn/ensemble/tests/test_gradient_boosting.py index 88e77751c2013..f18893b5d5744 100644 --- a/sklearn/ensemble/tests/test_gradient_boosting.py +++ b/sklearn/ensemble/tests/test_gradient_boosting.py @@ -84,13 +84,13 @@ def test_classification_toy(loss): ( {"learning_rate": "foo"}, TypeError, - "learning_rate must be an instance of ", + "learning_rate must be an instance of float", ), ({"n_estimators": 0}, ValueError, "n_estimators == 0, must be >= 1"), ( {"n_estimators": 1.5}, TypeError, - "n_estimators must be an instance of ,", + "n_estimators must be an instance of int,", ), ({"loss": "foobar"}, ValueError, "Loss 'foobar' not supported"), ({"subsample": 0.0}, ValueError, "subsample == 0.0, must be > 0.0"), @@ -98,7 +98,7 @@ def test_classification_toy(loss): ( {"subsample": "foo"}, TypeError, - "subsample must be an instance of ", + "subsample must be an instance of float", ), ({"init": {}}, ValueError, "The init parameter must be an estimator or 'zero'"), ({"max_features": 0}, ValueError, "max_features == 0, must be >= 1"), @@ -125,19 +125,19 @@ def test_classification_toy(loss): ( {"validation_fraction": "foo"}, TypeError, - "validation_fraction must be an instance of ", + "validation_fraction must be an instance of float", ), ({"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 must be an instance of int,", ), ({"tol": 0.0}, ValueError, "tol == 0.0, must be > 0.0"), ( {"tol": "foo"}, TypeError, - "tol must be an instance of ,", + "tol must be an instance of float,", ), # The following parameters are checked in BaseDecisionTree ({"min_samples_leaf": 0}, ValueError, "min_samples_leaf == 0, must be >= 1"), @@ -145,7 +145,7 @@ def test_classification_toy(loss): ( {"min_samples_leaf": "foo"}, TypeError, - "min_samples_leaf must be an instance of ", + "min_samples_leaf must be an instance of float", ), ({"min_samples_split": 1}, ValueError, "min_samples_split == 1, must be >= 2"), ( @@ -161,7 +161,7 @@ def test_classification_toy(loss): ( {"min_samples_split": "foo"}, TypeError, - "min_samples_split must be an instance of ", + "min_samples_split must be an instance of float", ), ( {"min_weight_fraction_leaf": -1}, @@ -176,19 +176,19 @@ def test_classification_toy(loss): ( {"min_weight_fraction_leaf": "foo"}, TypeError, - "min_weight_fraction_leaf must be an instance of ", + "min_weight_fraction_leaf must be an instance of float", ), ({"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 ", + "max_leaf_nodes must be an instance of int", ), ({"max_depth": -1}, ValueError, "max_depth == -1, must be >= 1"), ( {"max_depth": 1.1}, TypeError, - "max_depth must be an instance of ", + "max_depth must be an instance of int", ), ( {"min_impurity_decrease": -1}, @@ -198,13 +198,13 @@ def test_classification_toy(loss): ( {"min_impurity_decrease": "foo"}, TypeError, - "min_impurity_decrease must be an instance of ", + "min_impurity_decrease must be an instance of float", ), ({"ccp_alpha": -1.0}, ValueError, "ccp_alpha == -1.0, must be >= 0.0"), ( {"ccp_alpha": "foo"}, TypeError, - "ccp_alpha must be an instance of ", + "ccp_alpha must be an instance of float", ), ({"criterion": "mae"}, ValueError, "criterion='mae' is not supported."), ], From 34fa443e6453bf2e12c3ff56505998975638b524 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 9 Feb 2022 15:10:11 -0500 Subject: [PATCH 07/10] TST Add test for None --- sklearn/utils/tests/test_validation.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sklearn/utils/tests/test_validation.py b/sklearn/utils/tests/test_validation.py index d2b21cf78d463..97d6e7091f8ad 100644 --- a/sklearn/utils/tests/test_validation.py +++ b/sklearn/utils/tests/test_validation.py @@ -1108,6 +1108,15 @@ def test_check_scalar_valid(x): "neither", TypeError("test_name1 must be an instance of float, not int."), ), + ( + None, + "test_name1", + numbers.Real, + 2, + 4, + "neither", + TypeError("test_name1 must be an instance of float, not NoneType."), + ), ( 1, "test_name1", From a8622505e4f673c61c50f37693854571b9165f53 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Wed, 9 Feb 2022 15:12:40 -0500 Subject: [PATCH 08/10] CLN Less indentation --- sklearn/utils/validation.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/sklearn/utils/validation.py b/sklearn/utils/validation.py index 323aab8ae360c..5debaf27478db 100644 --- a/sklearn/utils/validation.py +++ b/sklearn/utils/validation.py @@ -1406,21 +1406,20 @@ def check_scalar( If `min_val`, `max_val` and `include_boundaries` are inconsistent. """ - if not isinstance(x, target_type): - - def type_name(t): - """Convert type into humman readable string.""" - module = t.__module__ - qualname = t.__qualname__ - if module == "builtins": - return qualname - elif module == "numbers": - if qualname == "Real": - return "float" - elif qualname == "Integral": - return "int" - return f"{module}.{qualname}" + def type_name(t): + """Convert type into humman readable string.""" + module = t.__module__ + qualname = t.__qualname__ + if module == "builtins": + return qualname + elif module == "numbers": + if qualname == "Real": + return "float" + elif qualname == "Integral": + return "int" + return f"{module}.{qualname}" + if not isinstance(x, target_type): if isinstance(target_type, tuple): types_str = ", ".join(type_name(t) for t in target_type) target_type_str = f"{{{types_str}}}" From 74cc346268db6b327f41374f4d4528d4f8680f58 Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Thu, 10 Feb 2022 16:44:30 -0500 Subject: [PATCH 09/10] TST Adds unit test for Integral --- sklearn/utils/tests/test_validation.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sklearn/utils/tests/test_validation.py b/sklearn/utils/tests/test_validation.py index 97d6e7091f8ad..b104d1721090b 100644 --- a/sklearn/utils/tests/test_validation.py +++ b/sklearn/utils/tests/test_validation.py @@ -1117,6 +1117,15 @@ def test_check_scalar_valid(x): "neither", TypeError("test_name1 must be an instance of float, not NoneType."), ), + ( + None, + "test_name1", + numbers.Integral, + 2, + 4, + "neither", + TypeError("test_name1 must be an instance of int, not NoneType."), + ), ( 1, "test_name1", From 57f965262a4f3b07d7b2f2fc102f83978157194b Mon Sep 17 00:00:00 2001 From: "Thomas J. Fan" Date: Thu, 10 Feb 2022 16:47:28 -0500 Subject: [PATCH 10/10] CLN Less indentation --- sklearn/utils/validation.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sklearn/utils/validation.py b/sklearn/utils/validation.py index 5debaf27478db..cf2265d5b21cd 100644 --- a/sklearn/utils/validation.py +++ b/sklearn/utils/validation.py @@ -1412,11 +1412,10 @@ def type_name(t): qualname = t.__qualname__ if module == "builtins": return qualname - elif module == "numbers": - if qualname == "Real": - return "float" - elif qualname == "Integral": - return "int" + elif t == numbers.Real: + return "float" + elif t == numbers.Integral: + return "int" return f"{module}.{qualname}" if not isinstance(x, target_type):