From a90b9824156eb38d3b6d71e1613e5ad16734addb Mon Sep 17 00:00:00 2001 From: jeremie du boisberranger Date: Wed, 1 Mar 2023 11:37:43 +0100 Subject: [PATCH 1/4] deprecate new instead of init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Brigitta Sipőcz --- doc/whats_new/v1.3.rst | 6 ++++++ sklearn/utils/deprecation.py | 11 +++++------ sklearn/utils/tests/test_deprecation.py | 12 +++++++++++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/doc/whats_new/v1.3.rst b/doc/whats_new/v1.3.rst index f89a711a447cf..15ed75292c4ce 100644 --- a/doc/whats_new/v1.3.rst +++ b/doc/whats_new/v1.3.rst @@ -268,6 +268,12 @@ Changelog during `transform` with no prior call to `fit` or `fit_transform`. :pr:`25190` by :user:`Vincent Maladière `. +- |API| A `FutureWarning` is now raised when instantiating a class which inherits from + a deprecated base class (i.e. decorated by :class:`utils.deprecated`) and which + overrides the `__init__` method. + :pr:`25733` by :user:`Brigitta Sipőcz ` and + :user:`Jérémie du Boisberranger `. + - |Enhancement| :func:`utils.multiclass.type_of_target` can identify pandas nullable data types as classification targets. :pr:`25638` by `Thomas Fan`_. diff --git a/sklearn/utils/deprecation.py b/sklearn/utils/deprecation.py index 19d41aa1eaf85..6a38824ba01bb 100644 --- a/sklearn/utils/deprecation.py +++ b/sklearn/utils/deprecation.py @@ -60,17 +60,16 @@ def _decorate_class(self, cls): if self.extra: msg += "; %s" % self.extra - # FIXME: we should probably reset __new__ for full generality - init = cls.__init__ + new = cls.__new__ def wrapped(*args, **kwargs): warnings.warn(msg, category=FutureWarning) - return init(*args, **kwargs) + return new(*args, **kwargs) - cls.__init__ = wrapped + cls.__new__ = wrapped - wrapped.__name__ = "__init__" - wrapped.deprecated_original = init + wrapped.__name__ = "__new__" + wrapped.deprecated_original = new return cls diff --git a/sklearn/utils/tests/test_deprecation.py b/sklearn/utils/tests/test_deprecation.py index b810cfb85d3f6..c5c56318d4cac 100644 --- a/sklearn/utils/tests/test_deprecation.py +++ b/sklearn/utils/tests/test_deprecation.py @@ -36,6 +36,13 @@ class MockClass4: pass +class MockClass5(MockClass1): + """Inherit from deprecated class but overwrite __init__""" + + def __init__(self): + pass + + @deprecated() def mock_function(): return 10 @@ -48,6 +55,8 @@ def test_deprecated(): MockClass2().method() with pytest.warns(FutureWarning, match="deprecated"): MockClass3() + with pytest.warns(FutureWarning, match="qwerty"): + MockClass5() with pytest.warns(FutureWarning, match="deprecated"): val = mock_function() assert val == 10 @@ -56,10 +65,11 @@ def test_deprecated(): def test_is_deprecated(): # Test if _is_deprecated helper identifies wrapping via deprecated # NOTE it works only for class methods and functions - assert _is_deprecated(MockClass1.__init__) + assert _is_deprecated(MockClass1.__new__) assert _is_deprecated(MockClass2().method) assert _is_deprecated(MockClass3.__init__) assert not _is_deprecated(MockClass4.__init__) + assert _is_deprecated(MockClass5.__new__) assert _is_deprecated(mock_function) From 16df0da73a9d8a3e4e3817ebe61ea666378c8c54 Mon Sep 17 00:00:00 2001 From: jeremiedbb Date: Fri, 3 Mar 2023 11:48:31 +0100 Subject: [PATCH 2/4] fix --- sklearn/tests/test_docstring_parameters.py | 9 ++++----- sklearn/utils/deprecation.py | 4 ++-- sklearn/utils/tests/test_deprecation.py | 6 +++--- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/sklearn/tests/test_docstring_parameters.py b/sklearn/tests/test_docstring_parameters.py index 274f83445ee7f..8bf3e5dd7b24a 100644 --- a/sklearn/tests/test_docstring_parameters.py +++ b/sklearn/tests/test_docstring_parameters.py @@ -109,12 +109,11 @@ def test_docstring_parameters(): "Error for __init__ of %s in %s:\n%s" % (cls, name, w[0]) ) - cls_init = getattr(cls, "__init__", None) - - if _is_deprecated(cls_init): + # Skip checks on deprecated classes + if _is_deprecated(cls.__new__): continue - elif cls_init is not None: - this_incorrect += check_docstring_parameters(cls.__init__, cdoc) + + this_incorrect += check_docstring_parameters(cls.__init__, cdoc) for method_name in cdoc.methods: method = getattr(cls, method_name) diff --git a/sklearn/utils/deprecation.py b/sklearn/utils/deprecation.py index 6a38824ba01bb..dafc83c4a4d7f 100644 --- a/sklearn/utils/deprecation.py +++ b/sklearn/utils/deprecation.py @@ -62,9 +62,9 @@ def _decorate_class(self, cls): new = cls.__new__ - def wrapped(*args, **kwargs): + def wrapped(cls, *args, **kwargs): warnings.warn(msg, category=FutureWarning) - return new(*args, **kwargs) + return new(cls) cls.__new__ = wrapped diff --git a/sklearn/utils/tests/test_deprecation.py b/sklearn/utils/tests/test_deprecation.py index c5c56318d4cac..e351ffe0c6bda 100644 --- a/sklearn/utils/tests/test_deprecation.py +++ b/sklearn/utils/tests/test_deprecation.py @@ -39,8 +39,8 @@ class MockClass4: class MockClass5(MockClass1): """Inherit from deprecated class but overwrite __init__""" - def __init__(self): - pass + def __init__(self, a): + self.a = a @deprecated() @@ -56,7 +56,7 @@ def test_deprecated(): with pytest.warns(FutureWarning, match="deprecated"): MockClass3() with pytest.warns(FutureWarning, match="qwerty"): - MockClass5() + MockClass5(42) with pytest.warns(FutureWarning, match="deprecated"): val = mock_function() assert val == 10 From 3a485bae089621f5c6c25097dfe3ae3f057aada5 Mon Sep 17 00:00:00 2001 From: jeremiedbb Date: Mon, 6 Mar 2023 13:56:07 +0100 Subject: [PATCH 3/4] safer wrapper --- sklearn/utils/deprecation.py | 4 +++- sklearn/utils/tests/test_deprecation.py | 13 ++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/sklearn/utils/deprecation.py b/sklearn/utils/deprecation.py index dafc83c4a4d7f..a5a70ed699197 100644 --- a/sklearn/utils/deprecation.py +++ b/sklearn/utils/deprecation.py @@ -64,7 +64,9 @@ def _decorate_class(self, cls): def wrapped(cls, *args, **kwargs): warnings.warn(msg, category=FutureWarning) - return new(cls) + if new is object.__new__: + return object.__new__(cls) + return new(cls, *args, **kwargs) cls.__new__ = wrapped diff --git a/sklearn/utils/tests/test_deprecation.py b/sklearn/utils/tests/test_deprecation.py index e351ffe0c6bda..98c69a8abb780 100644 --- a/sklearn/utils/tests/test_deprecation.py +++ b/sklearn/utils/tests/test_deprecation.py @@ -37,12 +37,21 @@ class MockClass4: class MockClass5(MockClass1): - """Inherit from deprecated class but overwrite __init__""" + """Inherit from deprecated class but does not call super().__init__.""" def __init__(self, a): self.a = a +@deprecated("a message") +class MockClass6: + """A deprecated class that overrides __new__.""" + + def __new__(cls, *args, **kwargs): + assert len(args) > 0 + return super().__new__(cls) + + @deprecated() def mock_function(): return 10 @@ -57,6 +66,8 @@ def test_deprecated(): MockClass3() with pytest.warns(FutureWarning, match="qwerty"): MockClass5(42) + with pytest.warns(FutureWarning, match="a message"): + MockClass6(42) with pytest.warns(FutureWarning, match="deprecated"): val = mock_function() assert val == 10 From c9a40bef14d81b4754f64d1272a1b8a0b7e5717b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20du=20Boisberranger?= <34657725+jeremiedbb@users.noreply.github.com> Date: Mon, 6 Mar 2023 17:23:29 +0100 Subject: [PATCH 4/4] fix merge conflict mistake --- doc/whats_new/v1.3.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/doc/whats_new/v1.3.rst b/doc/whats_new/v1.3.rst index a8414b0764e8e..76bf1024a232c 100644 --- a/doc/whats_new/v1.3.rst +++ b/doc/whats_new/v1.3.rst @@ -281,9 +281,6 @@ Changelog :pr:`25733` by :user:`Brigitta Sipőcz ` and :user:`Jérémie du Boisberranger `. -- |Enhancement| :func:`utils.multiclass.type_of_target` can identify pandas - nullable data types as classification targets. :pr:`25638` by `Thomas Fan`_. - :mod:`sklearn.semi_supervised` ..............................