Soften deprecation warning for **kwargs and add more guidance (#198)

Author: adriangb

docs/source/migration.rst

@@ -49,9 +49,17 @@ pass your loss function to the constructor:
 Variable keyword arguments in fit and predict
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-In a future release of SciKeras, variable keyword arguments (commonly referred to as
-``**kwargs``) will be removed from fit and predict. To future
-proof your code, you should instead declare these parameters in your constructor:
+Keras supports a variable keyword arguments (commonly referred to as ``**kwargs``) for ``fit`` and ``predict``.
+Scikit-Learn on the other hand does not support these arguments, and using them is largely incompatible with the Scikit-Learn ecosystem.
+As a compromise, SciKeras supports these arguments, but we recommended that you set parameters using the constructor
+or ``set_params`` for first-class SciKeras support.
+
+.. warning::
+
+   Passing keyword arguments to ``fit`` and ``predict`` is deprecated and will be removed in a future version of SciKeras.
+
+
+For example, to declare ``batch_size`` in the constructor:
 
 .. code:: diff
 
@@ -64,7 +72,19 @@ Or to declare separate values for ``fit`` and ``predict``:
 
 .. code:: python
 
-   clf = KerasClassifier(fit__batch_size=32, predict__batch_size=10000)
+   clf = KerasClassifier(..., fit__batch_size=32, predict__batch_size=10000)
+
+If you want to change the parameters on a live instance, you can do:
+
+.. code:: python
+
+   clf = KerasClassifier(...)
+   clf.set_params(fit__batch_size=32, predict__batch_size=10000)
+   clf.fit(...)
+
+Functionally, this is the same as passing these parameters to ``fit``, just with one more function call.
+This is much more compatible with the Scikit-Learn API.
+In fact, this is what Scikti-Learn does in the background for hyperparameter tuning.
 
 Renaming of ``build_fn`` to ``model``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -98,4 +118,4 @@ tunable parameters (i.e. settable via ``set_params``):
    + clf = KerasClassifier(get_model, model__my_param=123)  # option 2
 
 That said, if you do not need them to work with ``set_params`` (which is only really
-necessary if you are doing hyperparameter tuning), you do not need to make any changes.
+necessary if you are doing hyperparameter tuning), you do not need to make any changes.

scikeras/wrappers.py

@@ -1,7 +1,6 @@
 """Wrapper for using the Scikit-Learn API with Keras models.
 """
 import inspect
-import os
 import warnings
 
 from collections import defaultdict
@@ -36,12 +35,21 @@
 from scikeras.utils.transformers import ClassifierLabelEncoder, RegressorTargetEncoder
 
 
+_kwarg_warn = """Passing estimator parameters as keyword arguments (aka as `**kwargs`) to `{0}` is not supported by the Scikit-Learn API, and will be removed in a future version of SciKeras.
+
+To resolve this issue, either set these parameters in the constructor (e.g., `est = BaseWrapper(..., foo=bar)`) or via `set_params` (e.g., `est.set_params(foo=bar)`). The following parameters were passed to `{0}`:
+
+{1}
+
+More detail is available at https://www.adriangb.com/scikeras/migration.html#variable-keyword-arguments-in-fit-and-predict
+"""
+
+
 class BaseWrapper(BaseEstimator):
     """Implementation of the scikit-learn classifier API for Keras.
 
     Below are a list of SciKeras specific parameters. For details on other parameters,
-    please see the see the
-    [tf.keras.Model documentation](https://www.tensorflow.org/api_docs/python/tf/keras/Model).
+    please see the see the `tf.keras.Model documentation <https://www.tensorflow.org/api_docs/python/tf/keras/Model>`_.
 
     Parameters
     ----------
@@ -696,20 +704,23 @@ def fit(self, X, y, sample_weight=None, **kwargs) -> "BaseWrapper":
             If not provided, then each sample is given unit weight.
         **kwargs : Dict[str, Any]
             Extra arguments to route to ``Model.fit``.
-            This functionality has been deprecated, and will be removed in SciKeras 1.0.0.
-            These parameters can also be specified by prefixing `fit__` to a parameter at initialization;
-            e.g, `BaseWrapper(..., fit__batch_size=32, predict__batch_size=1000)`.
+
+        Warnings
+        --------
+            Passing estimator parameters as keyword arguments (aka as ``**kwargs``) to ``fit`` is not supported by the Scikit-Learn API,
+            and will be removed in a future version of SciKeras.
+            These parameters can also be specified by prefixing ``fit__`` to a parameter at initialization
+            (``BaseWrapper(..., fit__batch_size=32, predict__batch_size=1000)``)
+            or by using ``set_params`` (``est.set_params(fit__batch_size=32, predict__batch_size=1000)``).
+
         Returns
         -------
         BaseWrapper
-            A reference to the instance that can be chain called
-            (ex: instance.fit(X,y).transform(X) )
+            A reference to the instance that can be chain called (``est.fit(X,y).transform(X)``).
         """
-        for k, v in kwargs.items():
-            warnings.warn(
-                "``**kwargs`` has been deprecated in SciKeras 0.2.1 and support will be removed be 1.0.0."
-                f" Instead, set fit arguments at initialization (i.e., ``BaseWrapper({k}={v})``)"
-            )
+        if kwargs:
+            kwarg_list = "\n * ".join([f"`{k}={v}`" for k, v in kwargs.items()])
+            warnings.warn(_kwarg_warn.format("fit", kwarg_list))
 
         # epochs via kwargs > fit__epochs > epochs
         kwargs["epochs"] = kwargs.get(
@@ -886,11 +897,9 @@ def _predict_raw(self, X, **kwargs):
         For classification, this corresponds to predict_proba.
         For regression, this corresponds to predict.
         """
-        for k, v in kwargs.items():
-            warnings.warn(
-                "``**kwargs`` has been deprecated in SciKeras 0.2.1 and support will be removed be 1.0.0."
-                f" Instead, set predict arguments at initialization (i.e., ``BaseWrapper({k}={v})``)"
-            )
+        if kwargs:
+            kwarg_list = "\n * ".join([f"`{k}={v}`" for k, v in kwargs.items()])
+            warnings.warn(_kwarg_warn.format("predict", kwarg_list), stacklevel=2)
 
         # check if fitted
         if not self.initialized_:
@@ -925,9 +934,14 @@ def predict(self, X, **kwargs):
             and n_features is the number of features.
         **kwargs : Dict[str, Any]
             Extra arguments to route to ``Model.predict``.
-            This functionality has been deprecated, and will be removed in SciKeras 1.0.0.
-            These parameters can also be specified by prefixing `predict__` to a parameter at initialization;
-            e.g, `BaseWrapper(..., fit__batch_size=32, predict__batch_size=1000)`.
+
+        Warnings
+        --------
+            Passing estimator parameters as keyword arguments (aka as ``**kwargs``) to ``predict`` is not supported by the Scikit-Learn API,
+            and will be removed in a future version of SciKeras.
+            These parameters can also be specified by prefixing ``predict__`` to a parameter at initialization
+            (``BaseWrapper(..., fit__batch_size=32, predict__batch_size=1000)``)
+            or by using ``set_params`` (``est.set_params(fit__batch_size=32, predict__batch_size=1000)``).
 
         Returns
         -------
@@ -1090,8 +1104,7 @@ class KerasClassifier(BaseWrapper):
     """Implementation of the scikit-learn classifier API for Keras.
 
     Below are a list of SciKeras specific parameters. For details on other parameters,
-    please see the see the
-    [tf.keras.Model documentation](https://www.tensorflow.org/api_docs/python/tf/keras/Model).
+    please see the see the `tf.keras.Model documentation <https://www.tensorflow.org/api_docs/python/tf/keras/Model>`_.
 
     Parameters
     ----------
@@ -1351,15 +1364,19 @@ def fit(self, X, y, sample_weight=None, **kwargs) -> "KerasClassifier":
             If not provided, then each sample is given unit weight.
         **kwargs : Dict[str, Any]
             Extra arguments to route to ``Model.fit``.
-            This functionality has been deprecated, and will be removed in SciKeras 1.0.0.
-            These parameters can also be specified by prefixing `fit__` to a parameter at initialization;
-            e.g, `BaseWrapper(..., fit__batch_size=32, predict__batch_size=1000)`.
+
+        Warnings
+        --------
+            Passing estimator parameters as keyword arguments (aka as ``**kwargs``) to ``fit`` is not supported by the Scikit-Learn API,
+            and will be removed in a future version of SciKeras.
+            These parameters can also be specified by prefixing ``fit__`` to a parameter at initialization
+            (``KerasClassifier(..., fit__batch_size=32, predict__batch_size=1000)``)
+            or by using ``set_params`` (``est.set_params(fit__batch_size=32, predict__batch_size=1000)``).
 
         Returns
         -------
         KerasClassifier
-            A reference to the instance that can be chain called
-            (ex: instance.fit(X,y).transform(X) )
+            A reference to the instance that can be chain called (``est.fit(X,y).transform(X)``).
         """
         self.classes_ = None
         if self.class_weight is not None:
@@ -1415,9 +1432,14 @@ def predict_proba(self, X, **kwargs):
             and n_features is the number of features.
         **kwargs : Dict[str, Any]
             Extra arguments to route to ``Model.predict``.
-            This functionality has been deprecated, and will be removed in SciKeras 1.0.0.
-            These parameters can also be specified by prefixing `predict__` to a parameter at initialization;
-            e.g, `BaseWrapper(..., fit__batch_size=32, predict__batch_size=1000)`.
+
+        Warnings
+        --------
+            Passing estimator parameters as keyword arguments (aka as ``**kwargs``) to ``predict_proba`` is not supported by the Scikit-Learn API,
+            and will be removed in a future version of SciKeras.
+            These parameters can also be specified by prefixing ``predict__`` to a parameter at initialization
+            (``KerasClassifier(..., fit__batch_size=32, predict__batch_size=1000)``)
+            or by using ``set_params`` (``est.set_params(fit__batch_size=32, predict__batch_size=1000)``).
 
         Returns
         -------
@@ -1441,8 +1463,7 @@ class KerasRegressor(BaseWrapper):
     """Implementation of the scikit-learn classifier API for Keras.
 
     Below are a list of SciKeras specific parameters. For details on other parameters,
-    please see the see the
-    [tf.keras.Model documentation](https://www.tensorflow.org/api_docs/python/tf/keras/Model).
+    please see the see the `tf.keras.Model documentation <https://www.tensorflow.org/api_docs/python/tf/keras/Model>`_.
 
     Parameters
     ----------

tests/test_deprecation.py

@@ -1,13 +1,10 @@
 """Tests for features scheduled for deprecation.
 """
-from unittest import mock
-
-import numpy as np
 import pytest
 
-from scikeras.wrappers import KerasClassifier, KerasRegressor
+from scikeras.wrappers import KerasClassifier
 
-from .mlp_models import dynamic_classifier, dynamic_regressor
+from .mlp_models import dynamic_classifier
 
 
 def test_build_fn_deprecation():
@@ -17,72 +14,3 @@ def test_build_fn_deprecation():
     clf = KerasClassifier(build_fn=dynamic_classifier, model__hidden_layer_sizes=(100,))
     with pytest.warns(UserWarning, match="``build_fn`` will be renamed to ``model``"):
         clf.fit([[0], [1]], [0, 1])
-
-
-@pytest.mark.parametrize(
-    "wrapper,builder",
-    [(KerasClassifier, dynamic_classifier), (KerasRegressor, dynamic_regressor),],
-)
-def test_kwarg_deprecation(wrapper, builder):
-    """Test that SciKeras supports the **kwarg interface in fit and predict
-    but warns the user about deprecation of this interface.
-    """
-    original_batch_size = 128
-    kwarg_batch_size = 90
-    kwarg_epochs = (
-        2  # epochs is a special case for fit since SciKeras also uses it internally
-    )
-    extra_kwargs = {"workers": 1}  # chosen because it is not a SciKeras hardcoded param
-    est = wrapper(
-        model=builder,
-        model__hidden_layer_sizes=(100,),
-        warm_start=True,  # for mocking to work properly
-        batch_size=original_batch_size,  # test that this is overridden by kwargs
-        fit__batch_size=original_batch_size,  # test that this is overridden by kwargs
-        predict__batch_size=original_batch_size,  # test that this is overridden by kwargs
-    )
-    X, y = np.random.random((100, 10)), np.random.randint(low=0, high=3, size=(100,))
-    est.initialize(X, y)
-    match_txt = r"``\*\*kwargs`` has been deprecated in SciKeras"
-    # check fit
-    with pytest.warns(UserWarning, match=match_txt):
-        with mock.patch.object(
-            est.model_, "fit", side_effect=est.model_.fit
-        ) as mock_fit:
-            est.fit(
-                X, y, batch_size=kwarg_batch_size, epochs=kwarg_epochs, **extra_kwargs
-            )
-            call_args = mock_fit.call_args_list
-            assert len(call_args) == 1
-            call_kwargs = call_args[0][1]
-            assert "batch_size" in call_kwargs
-            assert call_kwargs["batch_size"] == kwarg_batch_size
-            assert call_kwargs["epochs"] == kwarg_epochs
-            assert len(est.history_["loss"]) == kwarg_epochs
-    # check predict
-    with pytest.warns(UserWarning, match=match_txt):
-        with mock.patch.object(
-            est.model_, "predict", side_effect=est.model_.predict
-        ) as mock_predict:
-            est.predict(X, batch_size=kwarg_batch_size, **extra_kwargs)
-            call_args = mock_predict.call_args_list
-            assert len(call_args) == 1
-            call_kwargs = call_args[0][1]
-            assert "batch_size" in call_kwargs
-            assert call_kwargs["batch_size"] == kwarg_batch_size
-            if isinstance(est, KerasClassifier):
-                est.predict_proba(X, batch_size=kwarg_batch_size, **extra_kwargs)
-                call_args = mock_predict.call_args_list
-                assert len(call_args) == 2
-                call_kwargs = call_args[1][1]
-                assert "batch_size" in call_kwargs
-                assert call_kwargs["batch_size"] == kwarg_batch_size
-    # check that params were restored and extra_kwargs were not stored
-    for param_name in ("batch_size", "fit__batch_size", "predict__batch_size"):
-        assert getattr(est, param_name) == original_batch_size
-    for k in extra_kwargs.keys():
-        assert (
-            not hasattr(est, k)
-            or hasattr(est, "fit__" + k)
-            or hasattr(est, "predict__" + k)
-        )

tests/test_parameters.py

@@ -1,5 +1,7 @@
 import os
 
+from unittest import mock
+
 import numpy as np
 import pytest
 
@@ -198,7 +200,7 @@ class TestMetricsParam:
     @pytest.mark.parametrize("metric", ("accuracy", "sparse_categorical_accuracy"))
     def test_metrics(self, metric):
         """Test the metrics param.
-        
+
         Specifically test ``accuracy``, which Keras automatically
         matches to the loss function and hence should be passed through
         as a string and not as a retrieved function.
@@ -252,3 +254,69 @@ def test_class_weight_param():
             clf.partial_fit(X_train, y_train)
         y_pred = clf.predict(X_test)
         assert np.mean(y_pred == 0) > 0.95
+
+
+@pytest.mark.parametrize(
+    "wrapper,builder",
+    [(KerasClassifier, dynamic_classifier), (KerasRegressor, dynamic_regressor)],
+)
+def test_kwargs(wrapper, builder):
+    """Test that SciKeras supports the **kwarg interface in fit and predict."""
+    original_batch_size = 128
+    kwarg_batch_size = 90
+    kwarg_epochs = (
+        2  # epochs is a special case for fit since SciKeras also uses it internally
+    )
+    extra_kwargs = {"workers": 1}  # chosen because it is not a SciKeras hardcoded param
+    est = wrapper(
+        model=builder,
+        model__hidden_layer_sizes=(100,),
+        warm_start=True,  # for mocking to work properly
+        batch_size=original_batch_size,  # test that this is overridden by kwargs
+        fit__batch_size=original_batch_size,  # test that this is overridden by kwargs
+        predict__batch_size=original_batch_size,  # test that this is overridden by kwargs
+    )
+    X, y = np.random.random((100, 10)), np.random.randint(low=0, high=3, size=(100,))
+    est.initialize(X, y)
+    # check fit
+    match = "estimator parameters as keyword arguments"
+    with mock.patch.object(est.model_, "fit", side_effect=est.model_.fit) as mock_fit:
+        with pytest.warns(UserWarning, match=match.format("fit")):
+            est.fit(
+                X, y, batch_size=kwarg_batch_size, epochs=kwarg_epochs, **extra_kwargs
+            )
+        call_args = mock_fit.call_args_list
+        assert len(call_args) == 1
+        call_kwargs = call_args[0][1]
+        assert "batch_size" in call_kwargs
+        assert call_kwargs["batch_size"] == kwarg_batch_size
+        assert call_kwargs["epochs"] == kwarg_epochs
+        assert len(est.history_["loss"]) == kwarg_epochs
+    # check predict
+    with mock.patch.object(
+        est.model_, "predict", side_effect=est.model_.predict
+    ) as mock_predict:
+        with pytest.warns(UserWarning, match=match.format("predict")):
+            est.predict(X, batch_size=kwarg_batch_size, **extra_kwargs)
+        call_args = mock_predict.call_args_list
+        assert len(call_args) == 1
+        call_kwargs = call_args[0][1]
+        assert "batch_size" in call_kwargs
+        assert call_kwargs["batch_size"] == kwarg_batch_size
+        if isinstance(est, KerasClassifier):
+            with pytest.warns(UserWarning, match=match.format("predict")):
+                est.predict_proba(X, batch_size=kwarg_batch_size, **extra_kwargs)
+            call_args = mock_predict.call_args_list
+            assert len(call_args) == 2
+            call_kwargs = call_args[1][1]
+            assert "batch_size" in call_kwargs
+            assert call_kwargs["batch_size"] == kwarg_batch_size
+    # check that params were restored and extra_kwargs were not stored
+    for param_name in ("batch_size", "fit__batch_size", "predict__batch_size"):
+        assert getattr(est, param_name) == original_batch_size
+    for k in extra_kwargs.keys():
+        assert (
+            not hasattr(est, k)
+            or hasattr(est, "fit__" + k)
+            or hasattr(est, "predict__" + k)
+        )