DEP Deprecate None in FeatureUnion (scikit-learn#15053)

thomasjpfan · adrinjalali · commit 16f4ac90f073 · 2019-10-06T11:08:27.000+02:00
* ENH Deprecates None in feature union * ENH Deprecates None in feature union * DOC Adds todo * DOC Adds TODO * STY * CLN Address @adrinjalali comments * DOC Adds whats_new * TST Better checks * CLN Nicer asserts * CLN Address nicholas comments * STY Fix flake8
diff --git a/doc/whats_new/v0.22.rst b/doc/whats_new/v0.22.rst
@@ -528,6 +528,10 @@ Changelog
   the final estimator does.
   :pr:`13806` by :user:`Anaël Beaugnon <ab-anssi>`.
 
+- |API| `None` as a transformer is now deprecated in
+  :class:`pipeline.FeatureUnion`. Please use `'drop'` instead. :pr:`15053` by
+  `Thomas Fan`_.
+
 - |Fix| The `fit` in :class:`~pipeline.FeatureUnion` now accepts `fit_params`
   to pass to the underlying transformers. :pr:`15119` by `Adrin Jalali`_.
 
diff --git a/sklearn/pipeline.py b/sklearn/pipeline.py
@@ -11,6 +11,7 @@
 
 from collections import defaultdict
 from itertools import islice
+import warnings
 
 import numpy as np
 from scipy import sparse
@@ -754,7 +755,7 @@ class FeatureUnion(TransformerMixin, _BaseComposition):
     Parameters of the transformers may be set using its name and the parameter
     name separated by a '__'. A transformer may be replaced entirely by
     setting the parameter with its name to another transformer,
-    or removed by setting to 'drop' or ``None``.
+    or removed by setting to 'drop'.
 
     Read more in the :ref:`User Guide <feature_union>`.
 
@@ -764,6 +765,9 @@ class FeatureUnion(TransformerMixin, _BaseComposition):
         List of transformer objects to be applied to the data. The first
         half of each tuple is the name of the transformer.
 
+        .. versionchanged:: 0.22
+           Deprecated `None` as a transformer in favor of 'drop'.
+
     n_jobs : int or None, optional (default=None)
         Number of jobs to run in parallel.
         ``None`` means 1 unless in a :obj:`joblib.parallel_backend` context.
@@ -840,7 +844,14 @@ def _validate_transformers(self):
 
         # validate estimators
         for t in transformers:
-            if t is None or t == 'drop':
+            # TODO: Remove in 0.24 when None is removed
+            if t is None:
+                warnings.warn("Using None as a transformer is deprecated "
+                              "in version 0.22 and will be removed in "
+                              "version 0.24. Please use 'drop' instead.",
+                              DeprecationWarning)
+                continue
+            if t == 'drop':
                 continue
             if (not (hasattr(t, "fit") or hasattr(t, "fit_transform")) or not
                     hasattr(t, "transform")):
diff --git a/sklearn/tests/test_pipeline.py b/sklearn/tests/test_pipeline.py
@@ -898,6 +898,7 @@ def test_set_feature_union_steps():
     assert ['mock__x5'] == ft.get_feature_names()
 
 
+# TODO: Remove parametrization in 0.24 when None is removed for FeatureUnion
 @pytest.mark.parametrize('drop', ['drop', None])
 def test_set_feature_union_step_drop(drop):
     mult2 = Mult(2)
@@ -911,25 +912,33 @@ def test_set_feature_union_step_drop(drop):
     assert_array_equal([[2, 3]], ft.fit_transform(X))
     assert ['m2__x2', 'm3__x3'] == ft.get_feature_names()
 
-    ft.set_params(m2=drop)
-    assert_array_equal([[3]], ft.fit(X).transform(X))
-    assert_array_equal([[3]], ft.fit_transform(X))
+    with pytest.warns(None) as record:
+        ft.set_params(m2=drop)
+        assert_array_equal([[3]], ft.fit(X).transform(X))
+        assert_array_equal([[3]], ft.fit_transform(X))
     assert ['m3__x3'] == ft.get_feature_names()
+    assert record if drop is None else not record
 
-    ft.set_params(m3=drop)
-    assert_array_equal([[]], ft.fit(X).transform(X))
-    assert_array_equal([[]], ft.fit_transform(X))
+    with pytest.warns(None) as record:
+        ft.set_params(m3=drop)
+        assert_array_equal([[]], ft.fit(X).transform(X))
+        assert_array_equal([[]], ft.fit_transform(X))
     assert [] == ft.get_feature_names()
-
-    # check we can change back
-    ft.set_params(m3=mult3)
-    assert_array_equal([[3]], ft.fit(X).transform(X))
-
-    # Check 'drop' step at construction time
-    ft = FeatureUnion([('m2', drop), ('m3', mult3)])
-    assert_array_equal([[3]], ft.fit(X).transform(X))
-    assert_array_equal([[3]], ft.fit_transform(X))
+    assert record if drop is None else not record
+
+    with pytest.warns(None) as record:
+        # check we can change back
+        ft.set_params(m3=mult3)
+        assert_array_equal([[3]], ft.fit(X).transform(X))
+    assert record if drop is None else not record
+
+    with pytest.warns(None) as record:
+        # Check 'drop' step at construction time
+        ft = FeatureUnion([('m2', drop), ('m3', mult3)])
+        assert_array_equal([[3]], ft.fit(X).transform(X))
+        assert_array_equal([[3]], ft.fit_transform(X))
     assert ['m3__x3'] == ft.get_feature_names()
+    assert record if drop is None else not record
 
 
 def test_step_name_validation():
@@ -1127,7 +1136,7 @@ def test_pipeline_param_error():
      (FeatureUnion([('mult1', Mult()), ('mult2', Mult())]),
       r'\[FeatureUnion\].*\(step 1 of 2\) Processing mult1.* total=.*\n'
       r'\[FeatureUnion\].*\(step 2 of 2\) Processing mult2.* total=.*\n$'),
-     (FeatureUnion([('mult1', None), ('mult2', Mult()), ('mult3', None)]),
+     (FeatureUnion([('mult1', 'drop'), ('mult2', Mult()), ('mult3', 'drop')]),
       r'\[FeatureUnion\].*\(step 1 of 1\) Processing mult2.* total=.*\n$')
     ], ['fit', 'fit_transform', 'fit_predict'])
     if hasattr(est, method) and not (
@@ -1173,3 +1182,16 @@ def transform(self, X, y=None):
 
     t.fit(X, y, a=0)
     t.fit_transform(X, y, a=0)
+
+
+# TODO: Remove in 0.24 when None is removed
+def test_feature_union_warns_with_none():
+    msg = (r"Using None as a transformer is deprecated in version 0\.22 and "
+           r"will be removed in version 0\.24\. Please use 'drop' instead\.")
+    with pytest.warns(DeprecationWarning, match=msg):
+        union = FeatureUnion([('multi1', None), ('multi2', Mult())])
+
+    X = [[1, 2, 3], [4, 5, 6]]
+
+    with pytest.warns(DeprecationWarning, match=msg):
+        union.fit_transform(X)
