From ba8a07325df0eb7d7d84fd1fa8f357898ef6092b Mon Sep 17 00:00:00 2001 From: Mathias Hauser Date: Fri, 12 Aug 2022 14:44:49 +0200 Subject: [PATCH 1/3] decorator to deprecate positional arguments --- licenses/SCIKIT_LEARN_LICENSE | 29 +++++++++ xarray/tests/test_deprecation_helpers.py | 80 ++++++++++++++++++++++++ xarray/util/deprecation_helpers.py | 75 ++++++++++++++++++++++ 3 files changed, 184 insertions(+) create mode 100644 licenses/SCIKIT_LEARN_LICENSE create mode 100644 xarray/tests/test_deprecation_helpers.py create mode 100644 xarray/util/deprecation_helpers.py diff --git a/licenses/SCIKIT_LEARN_LICENSE b/licenses/SCIKIT_LEARN_LICENSE new file mode 100644 index 00000000000..63bc7eebfb0 --- /dev/null +++ b/licenses/SCIKIT_LEARN_LICENSE @@ -0,0 +1,29 @@ +BSD 3-Clause License + +Copyright (c) 2007-2021 The scikit-learn developers. +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +* Neither the name of the copyright holder nor the names of its + contributors may be used to endorse or promote products derived from + this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE diff --git a/xarray/tests/test_deprecation_helpers.py b/xarray/tests/test_deprecation_helpers.py new file mode 100644 index 00000000000..714e81c6950 --- /dev/null +++ b/xarray/tests/test_deprecation_helpers.py @@ -0,0 +1,80 @@ +import pytest + +from xarray.util.deprecation_helpers import _deprecate_positional_args + + +def test_deprecate_positional_args_warns_for_function(): + @_deprecate_positional_args("v0.1") + def f1(a, b, *, c=1, d=1): + pass + + with pytest.warns(FutureWarning, match=r".*v0.1"): + f1(1, 2, 3) + + with pytest.warns(FutureWarning, match=r"Passing 'c' as positional"): + f1(1, 2, 3) + + with pytest.warns(FutureWarning, match=r"Passing 'c, d' as positional"): + f1(1, 2, 3, 4) + + @_deprecate_positional_args("v0.1") + def f2(a=1, *, b=1, c=1, d=1): + pass + + with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"): + f2(1, 2) + + @_deprecate_positional_args("v0.1") + def f3(a, *, b=1, **kwargs): + pass + + with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"): + f3(1, 2) + + with pytest.raises(TypeError, match=r"Cannot handle positional-only params"): + + @_deprecate_positional_args("v0.1") + def f4(a, /, *, b=2, **kwargs): + pass + + +def test_deprecate_positional_args_warns_for_class(): + class A1: + @_deprecate_positional_args("v0.1") + def __init__(self, a, b, *, c=1, d=1): + pass + + with pytest.warns(FutureWarning, match=r".*v0.1"): + A1(1, 2, 3) + + with pytest.warns(FutureWarning, match=r"Passing 'c' as positional"): + A1(1, 2, 3) + + with pytest.warns(FutureWarning, match=r"Passing 'c, d' as positional"): + A1(1, 2, 3, 4) + + class A2: + @_deprecate_positional_args("v0.1") + def __init__(self, a=1, b=1, *, c=1, d=1): + pass + + with pytest.warns(FutureWarning, match=r"Passing 'c' as positional"): + A2(1, 2, 3) + + with pytest.warns(FutureWarning, match=r"Passing 'c, d' as positional"): + A2(1, 2, 3, 4) + + class A3: + @_deprecate_positional_args("v0.1") + def __init__(self, a, *, b=1, **kwargs): + pass + + with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"): + A3(1, 2) + + with pytest.raises(TypeError, match=r"Cannot handle positional-only params"): + + class A3: + @_deprecate_positional_args("v0.1") + def __init__(self, a, /, *, b=1, **kwargs): + pass diff --git a/xarray/util/deprecation_helpers.py b/xarray/util/deprecation_helpers.py new file mode 100644 index 00000000000..e601ccb2787 --- /dev/null +++ b/xarray/util/deprecation_helpers.py @@ -0,0 +1,75 @@ +import inspect +import warnings +from functools import wraps + +POSITIONAL_OR_KEYWORD = inspect.Parameter.POSITIONAL_OR_KEYWORD +KEYWORD_ONLY = inspect.Parameter.KEYWORD_ONLY +POSITIONAL_ONLY = inspect.Parameter.POSITIONAL_ONLY + + +def _deprecate_positional_args(version): + """Decorator for methods that issues warnings for positional arguments + + Using the keyword-only argument syntax in pep 3102, arguments after the + ``*`` will issue a warning when passed as a positional argument. + + Parameters + ---------- + version : str + version of the library when the positional arguments were deprecated + + Examples + -------- + Deprecate passing `b` as positional argument: + + def func(a, b=1): + pass + + @_deprecate_positional_args("v0.1.0") + def func(a, *, b=2): + pass + + func(1, 2) + + Notes + ----- + This function is adapted from scikit-learn under the terms of its license. See + licences/SCIKIT_LEARN_LICENSE + """ + + def _decorator(f): + + signature = inspect.signature(f) + + pos_or_kw_args = [] + kwonly_args = [] + for name, param in signature.parameters.items(): + if param.kind == POSITIONAL_OR_KEYWORD: + pos_or_kw_args.append(name) + elif param.kind == KEYWORD_ONLY: + kwonly_args.append(name) + elif param.kind == POSITIONAL_ONLY: + raise TypeError("Cannot handle positional-only params") + # because all args are coverted to kwargs below + + @wraps(f) + def inner(*args, **kwargs): + n_extra_args = len(args) - len(pos_or_kw_args) + if n_extra_args > 0: + + extra_args = ", ".join(kwonly_args[:n_extra_args]) + + warnings.warn( + f"Passing '{extra_args}' as positional argument(s) " + f"was deprecated in version {version} and will raise an error two " + "releases later. Please pass them as keyword arguments." + "", + FutureWarning, + ) + + kwargs.update({name: arg for name, arg in zip(pos_or_kw_args, args)}) + return f(**kwargs) + + return inner + + return _decorator From cfb11cd25dbfcdbb2a9c4fac587df66785751d2d Mon Sep 17 00:00:00 2001 From: Mathias Hauser Date: Fri, 12 Aug 2022 17:50:29 +0200 Subject: [PATCH 2/3] disallow positional-only without default --- xarray/tests/test_deprecation_helpers.py | 13 +++++++++++++ xarray/util/deprecation_helpers.py | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/xarray/tests/test_deprecation_helpers.py b/xarray/tests/test_deprecation_helpers.py index 714e81c6950..4b7883aa7d9 100644 --- a/xarray/tests/test_deprecation_helpers.py +++ b/xarray/tests/test_deprecation_helpers.py @@ -37,6 +37,12 @@ def f3(a, *, b=1, **kwargs): def f4(a, /, *, b=2, **kwargs): pass + with pytest.raises(TypeError, match=r"Keyword-only param without default"): + + @_deprecate_positional_args("v0.1") + def f5(a, *, b, c=3, **kwargs): + pass + def test_deprecate_positional_args_warns_for_class(): class A1: @@ -78,3 +84,10 @@ class A3: @_deprecate_positional_args("v0.1") def __init__(self, a, /, *, b=1, **kwargs): pass + + with pytest.raises(TypeError, match=r"Keyword-only param without default"): + + class A4: + @_deprecate_positional_args("v0.1") + def __init__(self, a, *, b, c=3, **kwargs): + pass diff --git a/xarray/util/deprecation_helpers.py b/xarray/util/deprecation_helpers.py index e601ccb2787..0a8f7904af2 100644 --- a/xarray/util/deprecation_helpers.py +++ b/xarray/util/deprecation_helpers.py @@ -5,6 +5,7 @@ POSITIONAL_OR_KEYWORD = inspect.Parameter.POSITIONAL_OR_KEYWORD KEYWORD_ONLY = inspect.Parameter.KEYWORD_ONLY POSITIONAL_ONLY = inspect.Parameter.POSITIONAL_ONLY +EMPTY = inspect.Parameter.empty def _deprecate_positional_args(version): @@ -48,13 +49,20 @@ def _decorator(f): pos_or_kw_args.append(name) elif param.kind == KEYWORD_ONLY: kwonly_args.append(name) + if param.default is EMPTY: + # IMHO `def f(a, *, b):` does not make sense -> disallow it + # if removing this constraint -> need to add these to kwargs as well + raise TypeError("Keyword-only param without default disallowed.") elif param.kind == POSITIONAL_ONLY: raise TypeError("Cannot handle positional-only params") # because all args are coverted to kwargs below @wraps(f) def inner(*args, **kwargs): + print(f"{args=}") + print(f"{pos_or_kw_args=}") n_extra_args = len(args) - len(pos_or_kw_args) + print(f"{n_extra_args=}") if n_extra_args > 0: extra_args = ", ".join(kwonly_args[:n_extra_args]) @@ -66,8 +74,11 @@ def inner(*args, **kwargs): "", FutureWarning, ) + print(f"{kwargs=}") kwargs.update({name: arg for name, arg in zip(pos_or_kw_args, args)}) + print(f"{kwargs=}") + return f(**kwargs) return inner From 54043c8729fac7572f6ccd1c35c9b4ba39af1678 Mon Sep 17 00:00:00 2001 From: Mathias Hauser Date: Thu, 18 Aug 2022 17:30:33 +0200 Subject: [PATCH 3/3] add copyright notice [skip-ci] --- xarray/util/deprecation_helpers.py | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/xarray/util/deprecation_helpers.py b/xarray/util/deprecation_helpers.py index 0a8f7904af2..474c863bcd4 100644 --- a/xarray/util/deprecation_helpers.py +++ b/xarray/util/deprecation_helpers.py @@ -1,3 +1,36 @@ +# For reference, here is a copy of the scikit-learn copyright notice: + +# BSD 3-Clause License + +# Copyright (c) 2007-2021 The scikit-learn developers. +# All rights reserved. + +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: + +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. + +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. + +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. + +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE + + import inspect import warnings from functools import wraps