Repository: scikit-learn/scikit-learn
Pull request: FIX ignore *args and **kwargs in parameter validation of public functions
closes #25113 

Avoid including `*args` and `**kwargs` in the parameter validation.
Since the number of arguments is varying, we cannot create a constraint for them.
The idea is to filter them for the moment.

Problem statement:
Handling `*args` and `**kwargs` in parameter validation framework
Currently, our parameter validation framework is not designed to work properly with `*args` (I did not test it) and `**kwargs` passed in a function.

If `kwargs` is not part of `validate_params`, we raise an error as a missing constraint. If we add it in the constraint then `kwargs` is actually not the name of any parameter.

Hints from issue discussion:

Commit message: fix

Patch to review:
diff --git a/sklearn/model_selection/_split.py b/sklearn/model_selection/_split.py
index 61e2caa4ca7fa..476c1719f7b0d 100644
--- a/sklearn/model_selection/_split.py
+++ b/sklearn/model_selection/_split.py
@@ -28,6 +28,7 @@
 from ..utils.validation import _num_samples, column_or_1d
 from ..utils.validation import check_array
 from ..utils.multiclass import type_of_target
+from ..utils._param_validation import validate_params, Interval
 
 __all__ = [
     "BaseCrossValidator",
@@ -2460,6 +2461,23 @@ def check_cv(cv=5, y=None, *, classifier=False):
     return cv  # New style cv objects are passed without any modification
 
 
+@validate_params(
+    {
+        "test_size": [
+            Interval(numbers.Real, 0, 1, closed="neither"),
+            Interval(numbers.Integral, 0, None, closed="neither"),
+            None,
+        ],
+        "train_size": [
+            Interval(numbers.Real, 0, 1, closed="neither"),
+            Interval(numbers.Integral, 0, None, closed="neither"),
+            None,
+        ],
+        "random_state": ["random_state"],
+        "shuffle": ["boolean"],
+        "stratify": ["array-like", None],
+    }
+)
 def train_test_split(
     *arrays,
     test_size=None,
diff --git a/sklearn/model_selection/tests/test_split.py b/sklearn/model_selection/tests/test_split.py
index f502ebc8a3b6a..d8141c9aceb2a 100644
--- a/sklearn/model_selection/tests/test_split.py
+++ b/sklearn/model_selection/tests/test_split.py
@@ -1219,33 +1219,6 @@ def test_train_test_split_errors():
         train_test_split(range(10), train_size=11, test_size=1)
 
 
-@pytest.mark.parametrize(
-    "train_size,test_size",
-    [
-        (1.2, 0.8),
-        (1.0, 0.8),
-        (0.0, 0.8),
-        (-0.2, 0.8),
-        (0.8, 1.2),
-        (0.8, 1.0),
-        (0.8, 0.0),
-        (0.8, -0.2),
-    ],
-)
-def test_train_test_split_invalid_sizes1(train_size, test_size):
-    with pytest.raises(ValueError, match=r"should be .* in the \(0, 1\) range"):
-        train_test_split(range(10), train_size=train_size, test_size=test_size)
-
-
-@pytest.mark.parametrize(
-    "train_size,test_size",
-    [(-10, 0.8), (0, 0.8), (11, 0.8), (0.8, -10), (0.8, 0), (0.8, 11)],
-)
-def test_train_test_split_invalid_sizes2(train_size, test_size):
-    with pytest.raises(ValueError, match=r"should be either positive and smaller"):
-        train_test_split(range(10), train_size=train_size, test_size=test_size)
-
-
 @pytest.mark.parametrize(
     "train_size, exp_train, exp_test", [(None, 7, 3), (8, 8, 2), (0.8, 8, 2)]
 )
diff --git a/sklearn/tests/test_public_functions.py b/sklearn/tests/test_public_functions.py
index 62f9a14a59614..d4b18235e4bcf 100644
--- a/sklearn/tests/test_public_functions.py
+++ b/sklearn/tests/test_public_functions.py
@@ -17,6 +17,7 @@
     "sklearn.metrics.auc",
     "sklearn.metrics.mean_absolute_error",
     "sklearn.metrics.zero_one_loss",
+    "sklearn.model_selection.train_test_split",
     "sklearn.svm.l1_min_c",
 ]
 
@@ -39,8 +40,13 @@ def test_function_param_validation(func_module):
     parameter_constraints = getattr(func, "_skl_parameter_constraints")
 
     # generate valid values for the required parameters
+    # Generate valid values for the required parameters
+    # The parameters `*args` and `**kwargs` are ignored since we cannot generate
+    # constraints.
     required_params = [
-        p.name for p in func_sig.parameters.values() if p.default is p.empty
+        p.name
+        for p in func_sig.parameters.values()
+        if p.default is p.empty and p.kind not in (p.VAR_POSITIONAL, p.VAR_KEYWORD)
     ]
     valid_required_params = {}
     for param_name in required_params:


Code review comments:
diff_hunk: @@ -39,8 +40,13 @@ def test_function_param_validation(func_module):
     parameter_constraints = getattr(func, "_skl_parameter_constraints")
 
     # generate valid values for the required parameters
+    # Generate valid values for the required parameters
path: sklearn/tests/test_public_functions.py
text: @user1:
duplicated
```suggestion
```

diff_hunk: @@ -2460,6 +2461,23 @@ def check_cv(cv=5, y=None, *, classifier=False):
     return cv  # New style cv objects are passed without any modification
 
 
+@validate_params(
+    {
+        "test_size": [
+            Interval(numbers.Real, 0, 1, closed="neither"),
+            Interval(numbers.Integral, 0, None, closed="neither"),
path: sklearn/model_selection/_split.py
text: @user1:
Nitpick but when it's an interval of int I find more natural to include the bound when it's a closed bound
```suggestion
            Interval(numbers.Integral, 1, None, closed="left"),
```

diff_hunk: @@ -2460,6 +2461,23 @@ def check_cv(cv=5, y=None, *, classifier=False):
     return cv  # New style cv objects are passed without any modification
 
 
+@validate_params(
+    {
+        "test_size": [
+            Interval(numbers.Real, 0, 1, closed="neither"),
+            Interval(numbers.Integral, 0, None, closed="neither"),
+            None,
+        ],
+        "train_size": [
+            Interval(numbers.Real, 0, 1, closed="neither"),
+            Interval(numbers.Integral, 0, None, closed="neither"),
path: sklearn/model_selection/_split.py
text: @user1:
```suggestion
            Interval(numbers.Integral, 1, None, closed="left"),
```