[MRG] Fixes #8174: deprecation warning #8177

tzano · 2017-01-08T19:27:45Z

This fixes #8174.
As the parameter y should be deprecated from method transform and eventually removed in a future version. I have added the following warning message.

Transform() is used in the following files:

sklearn/cluster/birch.py:    def transform(self, X, y=None):
sklearn/cluster/k_means_.py:    def transform(self, X, y=None):
sklearn/decomposition/base.py:    def transform(self, X, y=None):
sklearn/decomposition/dict_learning.py:    def transform(self, X, y=None):
sklearn/decomposition/fastica_.py:    def transform(self, X, y=None, copy=True):
sklearn/decomposition/pca.py:    def transform(self, X, y=None):
sklearn/feature_extraction/dict_vectorizer.py:    def transform(self, X, y=None):
sklearn/feature_extraction/hashing.py:    def transform(self, raw_X, y=None):
sklearn/feature_extraction/text.py:    def transform(self, X, y=None):
sklearn/kernel_approximation.py:    def transform(self, X, y=None):
sklearn/kernel_approximation.py:    def transform(self, X, y=None):
sklearn/kernel_approximation.py:    def transform(self, X, y=None):
sklearn/neighbors/approximate.py:    def transform(self, X, y=None):
sklearn/preprocessing/_function_transformer.py:    def transform(self, X, y=None):
sklearn/preprocessing/data.py:    def transform(self, X, y=None, copy=None):
sklearn/preprocessing/data.py:    def transform(self, X, y=None):
sklearn/preprocessing/data.py:    def transform(self, X, y=None):
sklearn/preprocessing/data.py:    def transform(self, X, y=None):
sklearn/preprocessing/data.py:    def transform(self, X, y=None, copy=None):
sklearn/preprocessing/data.py:    def transform(self, X, y=None, copy=None):
sklearn/preprocessing/data.py:    def transform(self, K, y=None, copy=True):
sklearn/preprocessing/data.py:    def transform(self, X, y=None):
sklearn/random_projection.py:    def transform(self, X, y=None):

jnothman · 2017-01-08T21:43:13Z

Please do not modify anything other than what addresses this issue. Those cosmetic changes just make it harder to review.
Please check that y is not actually used, and note when it is so we can review.
Make y='deprecated' by default so that you can warn when it is explicitly set to None.
Will be removed in version 0.21 to be specific.

tzano · 2017-01-09T10:16:22Z

Thanks @jnothman.

** Modifications: **

Passed y='deprecated' to transform method, and changed warning message to:

    warnings.warn("The parameter y on transform() is "
                  "deprecated since 0.19 and will be removed in 0.21. ",
                  DeprecationWarning)

In the following methods, we don't use y.

sklearn/cluster/birch.py:    def transform(self, X, y='deprecated'):
sklearn/cluster/k_means_.py:    def transform(self, X, y='deprecated'):
sklearn/decomposition/base.py:    def transform(self, X, y='deprecated'):
sklearn/decomposition/dict_learning.py:    def transform(self, X, y='deprecated'):
sklearn/decomposition/fastica_.py:    def transform(self, X, y='deprecated', copy=True):
sklearn/decomposition/pca.py:    def transform(self, X, y='deprecated'):
sklearn/feature_extraction/dict_vectorizer.py:    def transform(self, X, y='deprecated'):
sklearn/feature_extraction/hashing.py:    def transform(self, raw_X, y='deprecated'):
sklearn/feature_extraction/text.py:    def transform(self, X, y='deprecated'):
sklearn/kernel_approximation.py:    def transform(self, X, y='deprecated'):
sklearn/kernel_approximation.py:    def transform(self, X, y='deprecated'):
sklearn/kernel_approximation.py:    def transform(self, X, y='deprecated'):
sklearn/neighbors/approximate.py:    def transform(self, X, y='deprecated'):
sklearn/preprocessing/data.py:    def transform(self, X, y='deprecated', copy=None):
sklearn/preprocessing/data.py:    def transform(self, X, y='deprecated'):
sklearn/preprocessing/data.py:    def transform(self, X, y='deprecated'):
sklearn/preprocessing/data.py:    def transform(self, X, y='deprecated'):
sklearn/preprocessing/data.py:    def transform(self, X, y='deprecated', copy=None):
sklearn/preprocessing/data.py:    def transform(self, X, y='deprecated', copy=None):
sklearn/preprocessing/data.py:    def transform(self, K, y='deprecated', copy=True):
sklearn/random_projection.py:    def transform(self, X, y='deprecated'):
sklearn/tests/test_base.py:        def transform(self, X, y='deprecated'):
sklearn/tests/test_pipeline.py:    def transform(self, X, y='deprecated'):
sklearn/tests/test_pipeline.py:    def transform(self, X, y='deprecated'):

The are two places where we make use of parameter y in transform method.

sklearn.preprocessing.FunctionTransformer.transform()

sklearn/preprocessing/_function_transformer.py: def transform(self, X, y=None):. However, this can be customized.

    def transform(self, X, y=None):
        return self._transform(X, y, self.func, self.kw_args)

    def inverse_transform(self, X, y=None):
        return self._transform(X, y, self.inverse_func, self.inv_kw_args)

    def _transform(self, X, y=None, func=None, kw_args=None):
        ... 
        return func(X, *((y,) if self.pass_y else ()),
                    **(kw_args if kw_args else {}))

This goes down to this part:

        return func(X, *((y,) if self.pass_y else ()),
                    **(kw_args if kw_args else {}))

If we are intending to deprecate y from FunctionTransformer.transform(), it seems that we need to change FunctionTransformer (i.e: self.pass_y: wouldn't be needed. we can pass y with self.kw_args ). If it's the case, should we adjust FunctionTransformer class, or should we keep the same behaviour, for the sake of backwards compatibility.?

sklearn.cross_decomposition_
You didn't mention this in the initial list sklearn/cross_decomposition/pls_.py: def transform(self, X, Y=None, copy=True):

jnothman · 2017-01-09T11:21:43Z

Don't modify FunctionTransformer, thanks. I'd like to hear further on @GaelVaroquaux's concerns about this task.

jnothman · 2017-01-09T11:22:43Z

sklearn/decomposition/tests/test_pca.py

+        assert_warns_message(DeprecationWarning, warning_msg, X_r.transform,
+                        X, y=1.0)
+
+        warning_msg = "The parameter y on transform() is deprecated"


this variable is already set

jnothman · 2017-01-09T11:22:52Z

sklearn/decomposition/tests/test_pca.py

+                        X, y=1.0)
+
+        warning_msg = "The parameter y on transform() is deprecated"
+        X_r = pca.fit(X)


this need not be repeated

jnothman · 2017-01-09T11:23:26Z

sklearn/preprocessing/_function_transformer.py

@@ -78,6 +81,11 @@ def fit(self, X, y=None):
        return self

    def transform(self, X, y=None):


'deprecated'

jnothman · 2017-01-09T11:23:32Z

sklearn/preprocessing/_function_transformer.py

@@ -78,6 +81,11 @@ def fit(self, X, y=None):
        return self

    def transform(self, X, y=None):
+        if y is not None:
+            warnings.warn('y is deprecated and will be'
+                          ' removed in a future version',


tzano · 2017-01-11T08:49:53Z

@jnothman I've made the changes. Let me know if we need to add/remove the deprecation warning message from
sklearn/preprocessing/_function_transformer.py: def transform(self, X, y=None). I hope also that @GaelVaroquaux has a moment to chime in.

jnothman · 2017-01-11T10:33:04Z

Do not deprecate y in FunctionTransformer.

…

On 11 January 2017 at 19:49, Tahar ***@***.***> wrote: @jnothman <https://github.com/jnothman> I've made the changes. Let me know if we need to add/remove the deprecation warning message from sklearn/preprocessing/_function_transformer.py: def transform(self, X, y=None). I hope also that @GaelVaroquaux <https://github.com/GaelVaroquaux> has a moment to chime in. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8177 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz6w7yImOo24TquutAla_rVNgq-mQiks5rRJezgaJpZM4LdxcZ> .

tzano · 2017-01-11T17:32:39Z

Thanks @jnothman. I have pushed the changes.

Fixes #8174

90c6623

tzano changed the title ~~[WIP] Fixes #8174: deprecated warning~~ [WIP] Fixes #8174: deprecation warning Jan 8, 2017

jnothman mentioned this pull request Jan 8, 2017

transform should not accept y #8174

Closed

jnothman reviewed Jan 9, 2017

View reviewed changes

tzano changed the title ~~[WIP] Fixes #8174: deprecation warning~~ [MRG] Fixes #8174: deprecation warning Jan 14, 2017

tzano added 3 commits January 19, 2017 00:14

Fixes #8174: added some updates

a0d60ef

Fixes #8174: added some updates

90a033c

Sync changes

91216bd

tzano closed this Feb 10, 2017

lesteve mentioned this pull request Feb 21, 2017

[MRG+1] Deprecate Y parameter on transform() #8403

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Fixes #8174: deprecation warning #8177

[MRG] Fixes #8174: deprecation warning #8177

tzano commented Jan 8, 2017 •

edited

Loading

jnothman commented Jan 8, 2017

tzano commented Jan 9, 2017

jnothman commented Jan 9, 2017

jnothman Jan 9, 2017

jnothman Jan 9, 2017

jnothman Jan 9, 2017

jnothman Jan 9, 2017

tzano commented Jan 11, 2017

jnothman commented Jan 11, 2017 via email

tzano commented Jan 11, 2017

		@@ -78,6 +81,11 @@ def fit(self, X, y=None):
		return self

		def transform(self, X, y=None):

[MRG] Fixes #8174: deprecation warning #8177

[MRG] Fixes #8174: deprecation warning #8177

Conversation

tzano commented Jan 8, 2017 • edited Loading

jnothman commented Jan 8, 2017

tzano commented Jan 9, 2017

jnothman commented Jan 9, 2017

jnothman Jan 9, 2017

Choose a reason for hiding this comment

jnothman Jan 9, 2017

Choose a reason for hiding this comment

jnothman Jan 9, 2017

Choose a reason for hiding this comment

jnothman Jan 9, 2017

Choose a reason for hiding this comment

tzano commented Jan 11, 2017

jnothman commented Jan 11, 2017 via email

tzano commented Jan 11, 2017

tzano commented Jan 8, 2017 •

edited

Loading