fix _deprecate_positional_args helper (#6967)

* fix _deprecate_positional_args helper

* clean tests

Author: mathause

xarray/tests/test_deprecation_helpers.py

@@ -5,37 +5,60 @@
 
 def test_deprecate_positional_args_warns_for_function():
     @_deprecate_positional_args("v0.1")
-    def f1(a, b, *, c=1, d=1):
-        pass
+    def f1(a, b, *, c="c", d="d"):
+        return a, b, c, d
+
+    result = f1(1, 2)
+    assert result == (1, 2, "c", "d")
+
+    result = f1(1, 2, c=3, d=4)
+    assert result == (1, 2, 3, 4)
 
     with pytest.warns(FutureWarning, match=r".*v0.1"):
-        f1(1, 2, 3)
+        result = f1(1, 2, 3)
+    assert result == (1, 2, 3, "d")
 
     with pytest.warns(FutureWarning, match=r"Passing 'c' as positional"):
-        f1(1, 2, 3)
+        result = f1(1, 2, 3)
+    assert result == (1, 2, 3, "d")
 
     with pytest.warns(FutureWarning, match=r"Passing 'c, d' as positional"):
-        f1(1, 2, 3, 4)
+        result = f1(1, 2, 3, 4)
+    assert result == (1, 2, 3, 4)
 
     @_deprecate_positional_args("v0.1")
-    def f2(a=1, *, b=1, c=1, d=1):
-        pass
+    def f2(a="a", *, b="b", c="c", d="d"):
+        return a, b, c, d
 
     with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"):
-        f2(1, 2)
+        result = f2(1, 2)
+    assert result == (1, 2, "c", "d")
 
     @_deprecate_positional_args("v0.1")
-    def f3(a, *, b=1, **kwargs):
-        pass
+    def f3(a, *, b="b", **kwargs):
+        return a, b, kwargs
 
     with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"):
-        f3(1, 2)
+        result = f3(1, 2)
+    assert result == (1, 2, {})
 
-    with pytest.raises(TypeError, match=r"Cannot handle positional-only params"):
+    with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"):
+        result = f3(1, 2, f="f")
+    assert result == (1, 2, {"f": "f"})
 
-        @_deprecate_positional_args("v0.1")
-        def f4(a, /, *, b=2, **kwargs):
-            pass
+    @_deprecate_positional_args("v0.1")
+    def f4(a, /, *, b="b", **kwargs):
+        return a, b, kwargs
+
+    result = f4(1)
+    assert result == (1, "b", {})
+
+    result = f4(1, b=2, f="f")
+    assert result == (1, 2, {"f": "f"})
+
+    with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"):
+        result = f4(1, 2, f="f")
+    assert result == (1, 2, {"f": "f"})
 
     with pytest.raises(TypeError, match=r"Keyword-only param without default"):
 
@@ -47,47 +70,71 @@ def f5(a, *, b, c=3, **kwargs):
 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
+        def method(self, a, b, *, c="c", d="d"):
+            return a, b, c, d
+
+    result = A1().method(1, 2)
+    assert result == (1, 2, "c", "d")
+
+    result = A1().method(1, 2, c=3, d=4)
+    assert result == (1, 2, 3, 4)
 
     with pytest.warns(FutureWarning, match=r".*v0.1"):
-        A1(1, 2, 3)
+        result = A1().method(1, 2, 3)
+    assert result == (1, 2, 3, "d")
 
     with pytest.warns(FutureWarning, match=r"Passing 'c' as positional"):
-        A1(1, 2, 3)
+        result = A1().method(1, 2, 3)
+    assert result == (1, 2, 3, "d")
 
     with pytest.warns(FutureWarning, match=r"Passing 'c, d' as positional"):
-        A1(1, 2, 3, 4)
+        result = A1().method(1, 2, 3, 4)
+    assert result == (1, 2, 3, 4)
 
     class A2:
         @_deprecate_positional_args("v0.1")
-        def __init__(self, a=1, b=1, *, c=1, d=1):
-            pass
+        def method(self, a=1, b=1, *, c="c", d="d"):
+            return a, b, c, d
 
     with pytest.warns(FutureWarning, match=r"Passing 'c' as positional"):
-        A2(1, 2, 3)
+        result = A2().method(1, 2, 3)
+    assert result == (1, 2, 3, "d")
 
     with pytest.warns(FutureWarning, match=r"Passing 'c, d' as positional"):
-        A2(1, 2, 3, 4)
+        result = A2().method(1, 2, 3, 4)
+    assert result == (1, 2, 3, 4)
 
     class A3:
         @_deprecate_positional_args("v0.1")
-        def __init__(self, a, *, b=1, **kwargs):
-            pass
+        def method(self, a, *, b="b", **kwargs):
+            return a, b, kwargs
 
     with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"):
-        A3(1, 2)
+        result = A3().method(1, 2)
+    assert result == (1, 2, {})
 
-    with pytest.raises(TypeError, match=r"Cannot handle positional-only params"):
+    with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"):
+        result = A3().method(1, 2, f="f")
+    assert result == (1, 2, {"f": "f"})
 
-        class A3:
-            @_deprecate_positional_args("v0.1")
-            def __init__(self, a, /, *, b=1, **kwargs):
-                pass
+    class A4:
+        @_deprecate_positional_args("v0.1")
+        def method(self, a, /, *, b="b", **kwargs):
+            return a, b, kwargs
+
+    result = A4().method(1)
+    assert result == (1, "b", {})
+
+    result = A4().method(1, b=2, f="f")
+    assert result == (1, 2, {"f": "f"})
+
+    with pytest.warns(FutureWarning, match=r"Passing 'b' as positional"):
+        result = A4().method(1, 2, f="f")
+    assert result == (1, 2, {"f": "f"})
 
     with pytest.raises(TypeError, match=r"Keyword-only param without default"):
 
-        class A4:
+        class A5:
             @_deprecate_positional_args("v0.1")
             def __init__(self, a, *, b, c=3, **kwargs):
                 pass

xarray/util/deprecation_helpers.py

@@ -71,43 +71,46 @@ def func(a, *, b=2):
     licences/SCIKIT_LEARN_LICENSE
     """
 
-    def _decorator(f):
+    def _decorator(func):
 
-        signature = inspect.signature(f)
+        signature = inspect.signature(func)
 
         pos_or_kw_args = []
         kwonly_args = []
         for name, param in signature.parameters.items():
-            if param.kind == POSITIONAL_OR_KEYWORD:
+            if param.kind in (POSITIONAL_OR_KEYWORD, POSITIONAL_ONLY):
                 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)
+        @wraps(func)
         def inner(*args, **kwargs):
+
+            name = func.__name__
             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"Passing '{extra_args}' as positional argument(s) to {name} "
                     f"was deprecated in version {version} and will raise an error two "
                     "releases later. Please pass them as keyword arguments."
                     "",
                     FutureWarning,
+                    stacklevel=2,
                 )
 
-            kwargs.update({name: arg for name, arg in zip(pos_or_kw_args, args)})
+                zip_args = zip(kwonly_args[:n_extra_args], args[-n_extra_args:])
+                kwargs.update({name: arg for name, arg in zip_args})
+
+                return func(*args[:-n_extra_args], **kwargs)
 
-            return f(**kwargs)
+            return func(*args, **kwargs)
 
         return inner