Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Make deprecate_nonkeyword_arguments alter function signature #48693

Merged
merged 10 commits into from
Sep 26, 2022
17 changes: 17 additions & 0 deletions pandas/tests/util/test_deprecate_nonkeyword_arguments.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Tests for the `deprecate_nonkeyword_arguments` decorator
"""

import inspect
import warnings

from pandas.util._decorators import deprecate_nonkeyword_arguments
Expand All @@ -16,6 +17,10 @@ def f(a, b=0, c=0, d=0):
return a + b + c + d


def test_f_signature():
assert str(inspect.signature(f)) == "(a, b=0, *, c=0, d=0)"


def test_one_argument():
with tm.assert_produces_warning(None):
assert f(19) == 19
Expand Down Expand Up @@ -65,6 +70,10 @@ def g(a, b=0, c=0, d=0):
return a + b + c + d


def test_g_signature():
assert str(inspect.signature(g)) == "(a, *, b=0, c=0, d=0)"


def test_one_and_three_arguments_default_allowed_args():
with tm.assert_produces_warning(None):
assert g(1, b=3, c=3, d=5) == 12
Expand Down Expand Up @@ -93,6 +102,10 @@ def h(a=0, b=0, c=0, d=0):
return a + b + c + d


def test_h_signature():
assert str(inspect.signature(h)) == "(*, a=0, b=0, c=0, d=0)"


def test_all_keyword_arguments():
with tm.assert_produces_warning(None):
assert h(a=1, b=2) == 3
Expand Down Expand Up @@ -122,6 +135,10 @@ def baz(self, bar=None, foobar=None):
...


def test_foo_signature():
assert str(inspect.signature(Foo.baz)) == "(self, bar=None, *, foobar=None)"


def test_class():
msg = (
r"In a future version of pandas all arguments of Foo\.baz "
Expand Down
27 changes: 21 additions & 6 deletions pandas/util/_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,29 @@ def deprecate_nonkeyword_arguments(
"""

def decorate(func):
old_sig = inspect.signature(func)

if allowed_args is not None:
allow_args = allowed_args
else:
spec = inspect.getfullargspec(func)
allow_args = [
p.name
for p in old_sig.parameters.values()
if p.kind in (p.POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test that requires p.POSITIONAL_ONLY? I tried deleting it from here, and pytest pandas/tests/util/test_deprecate_nonkeyword_arguments.py still passed

If there's not a test that needs this, could we add a little one that does?

EDIT: apologies, I wrote this comment then forgot to add this comment to the previous review

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test!

and p.default is p.empty
]

# We must have some defaults if we are deprecating default-less
assert spec.defaults is not None # for mypy
allow_args = spec.args[: -len(spec.defaults)]
new_params = [
p.replace(kind=p.KEYWORD_ONLY)
if (
p.kind in (p.POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD)
and p.name not in allow_args
)
else p
for p in old_sig.parameters.values()
]
new_params.sort(key=lambda p: p.kind)
new_sig = old_sig.replace(parameters=new_params)

num_allow_args = len(allow_args)
msg = (
Expand All @@ -307,15 +322,15 @@ def decorate(func):

@wraps(func)
def wrapper(*args, **kwargs):
arguments = _format_argument_list(allow_args)
if len(args) > num_allow_args:
warnings.warn(
msg.format(arguments=arguments),
msg.format(arguments=_format_argument_list(allow_args)),
MarcoGorelli marked this conversation as resolved.
Show resolved Hide resolved
FutureWarning,
stacklevel=find_stack_level(inspect.currentframe()),
)
return func(*args, **kwargs)

wrapper.__signature__ = new_sig # type: ignore[attr-defined]
hauntsaninja marked this conversation as resolved.
Show resolved Hide resolved
return wrapper

return decorate
Expand Down