From 6df71310d7ada8e8ffb2c056bd8405992ceb225d Mon Sep 17 00:00:00 2001 From: Gleb Levitski <36483986+glevv@users.noreply.github.com> Date: Tue, 8 Nov 2022 20:00:55 +0000 Subject: [PATCH 1/7] added parameter validation for l1_min_c function --- sklearn/svm/_bounds.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sklearn/svm/_bounds.py b/sklearn/svm/_bounds.py index f89b6a77fc616..abbb52af15d00 100644 --- a/sklearn/svm/_bounds.py +++ b/sklearn/svm/_bounds.py @@ -7,8 +7,19 @@ from ..preprocessing import LabelBinarizer from ..utils.validation import check_consistent_length, check_array from ..utils.extmath import safe_sparse_dot +from ..utils._param_validation import StrOptions +from ..utils._param_validation import validate_params +@validate_params( + { + "X": ["array-like", "sparse matrix"], + "y": "array-like", + "loss": StrOptions({"squared_hinge", "log"}), + "fit_intercept": "boolean", + "intercept_scaling": float, + } +) def l1_min_c(X, y, *, loss="squared_hinge", fit_intercept=True, intercept_scaling=1.0): """Return the lowest bound for C. @@ -49,8 +60,6 @@ def l1_min_c(X, y, *, loss="squared_hinge", fit_intercept=True, intercept_scalin l1_min_c : float Minimum value for C. """ - if loss not in ("squared_hinge", "log"): - raise ValueError('loss type not in ("squared_hinge", "log")') X = check_array(X, accept_sparse="csc") check_consistent_length(X, y) From bb3256a4d0d5b3f4d958ad75facd7a7bd93b6026 Mon Sep 17 00:00:00 2001 From: Gleb Levitski <36483986+glevv@users.noreply.github.com> Date: Tue, 8 Nov 2022 20:05:01 +0000 Subject: [PATCH 2/7] added to validation list --- sklearn/tests/test_public_functions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sklearn/tests/test_public_functions.py b/sklearn/tests/test_public_functions.py index d4e645c052dab..6ea4f5ce73f67 100644 --- a/sklearn/tests/test_public_functions.py +++ b/sklearn/tests/test_public_functions.py @@ -10,6 +10,7 @@ PARAM_VALIDATION_FUNCTION_LIST = [ "sklearn.cluster.kmeans_plusplus", + "sklearn.svm.l1_min_c", ] From 7f76300da3153e15916c109a9a4acd75b369438d Mon Sep 17 00:00:00 2001 From: Gleb Levitski <36483986+glevv@users.noreply.github.com> Date: Wed, 9 Nov 2022 00:16:51 +0300 Subject: [PATCH 3/7] fix brackets --- sklearn/svm/_bounds.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sklearn/svm/_bounds.py b/sklearn/svm/_bounds.py index abbb52af15d00..88d875fd005c2 100644 --- a/sklearn/svm/_bounds.py +++ b/sklearn/svm/_bounds.py @@ -14,10 +14,10 @@ @validate_params( { "X": ["array-like", "sparse matrix"], - "y": "array-like", - "loss": StrOptions({"squared_hinge", "log"}), - "fit_intercept": "boolean", - "intercept_scaling": float, + "y": ["array-like"], + "loss": [StrOptions({"squared_hinge", "log"})], + "fit_intercept": ["boolean"], + "intercept_scaling": [float], } ) def l1_min_c(X, y, *, loss="squared_hinge", fit_intercept=True, intercept_scaling=1.0): From 8bd4d81ac7f3e2f93024c0fbc57e01d17f4c34a0 Mon Sep 17 00:00:00 2001 From: Gleb Levitski <36483986+glevv@users.noreply.github.com> Date: Wed, 9 Nov 2022 00:33:36 +0300 Subject: [PATCH 4/7] change val params for intercept scaling --- sklearn/svm/_bounds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/svm/_bounds.py b/sklearn/svm/_bounds.py index 88d875fd005c2..f0b0c6e864519 100644 --- a/sklearn/svm/_bounds.py +++ b/sklearn/svm/_bounds.py @@ -17,7 +17,7 @@ "y": ["array-like"], "loss": [StrOptions({"squared_hinge", "log"})], "fit_intercept": ["boolean"], - "intercept_scaling": [float], + "intercept_scaling": [Interval(Real, 0, None, closed="neither")], } ) def l1_min_c(X, y, *, loss="squared_hinge", fit_intercept=True, intercept_scaling=1.0): From 16f5f8b9e727851fc371cdef7e7adf974241ccf5 Mon Sep 17 00:00:00 2001 From: Gleb Levitski <36483986+glevv@users.noreply.github.com> Date: Wed, 9 Nov 2022 00:44:52 +0300 Subject: [PATCH 5/7] fixed imports --- sklearn/svm/_bounds.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sklearn/svm/_bounds.py b/sklearn/svm/_bounds.py index f0b0c6e864519..83cb72d30892c 100644 --- a/sklearn/svm/_bounds.py +++ b/sklearn/svm/_bounds.py @@ -2,13 +2,14 @@ # Author: Paolo Losi # License: BSD 3 clause +from numbers import Real + import numpy as np from ..preprocessing import LabelBinarizer from ..utils.validation import check_consistent_length, check_array from ..utils.extmath import safe_sparse_dot -from ..utils._param_validation import StrOptions -from ..utils._param_validation import validate_params +from ..utils._param_validation import StrOptions, Interval, validate_params @validate_params( From a35e9f312c7b3bd5bdb03a58322e6ee01acb8c38 Mon Sep 17 00:00:00 2001 From: Gleb Levitski <36483986+glevv@users.noreply.github.com> Date: Wed, 9 Nov 2022 07:36:54 +0300 Subject: [PATCH 6/7] updated tests --- sklearn/svm/tests/test_bounds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sklearn/svm/tests/test_bounds.py b/sklearn/svm/tests/test_bounds.py index 5ca0f12b5d7f0..541bf37cc80bf 100644 --- a/sklearn/svm/tests/test_bounds.py +++ b/sklearn/svm/tests/test_bounds.py @@ -37,7 +37,7 @@ def test_l1_min_c(loss, X_label, Y_label, intercept_label): def test_l1_min_c_l2_loss(): # loss='l2' should raise ValueError - msg = "loss type not in" + msg = "must be a str among" with pytest.raises(ValueError, match=msg): l1_min_c(dense_X, Y1, loss="l2") From 1eb260d4637d0e957cf311c8bda9521cd54a876d Mon Sep 17 00:00:00 2001 From: Gleb Levitski <36483986+glevv@users.noreply.github.com> Date: Wed, 9 Nov 2022 07:54:06 +0300 Subject: [PATCH 7/7] removed unnecessary tests --- sklearn/svm/tests/test_bounds.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/sklearn/svm/tests/test_bounds.py b/sklearn/svm/tests/test_bounds.py index 541bf37cc80bf..23d6be2f44e98 100644 --- a/sklearn/svm/tests/test_bounds.py +++ b/sklearn/svm/tests/test_bounds.py @@ -35,13 +35,6 @@ def test_l1_min_c(loss, X_label, Y_label, intercept_label): check_l1_min_c(X, Y, loss, **intercept_params) -def test_l1_min_c_l2_loss(): - # loss='l2' should raise ValueError - msg = "must be a str among" - with pytest.raises(ValueError, match=msg): - l1_min_c(dense_X, Y1, loss="l2") - - def check_l1_min_c(X, y, loss, fit_intercept=True, intercept_scaling=1.0): min_c = l1_min_c( X, @@ -76,11 +69,6 @@ def test_ill_posed_min_c(): l1_min_c(X, y) -def test_unsupported_loss(): - with pytest.raises(ValueError): - l1_min_c(dense_X, Y1, loss="l1") - - _MAX_UNSIGNED_INT = 4294967295