-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: Make deprecate_nonkeyword_arguments alter function signature #48693
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
Changes from 7 commits
4b35e7c
d720f62
44014ed
f642a04
f60bd70
8b3e68c
d3d912b
b39ae26
5c4836a
75769a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a test that requires 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 = ( | ||
|
@@ -307,15 +322,17 @@ 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) | ||
|
||
# error: "Callable[[VarArg(Any), KwArg(Any)], Any]" has no | ||
# attribute "__signature__" | ||
wrapper.__signature__ = new_sig # type: ignore[attr-defined] | ||
hauntsaninja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return wrapper | ||
|
||
return decorate | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MarcoGorelli Should this go in 1.5.1 if this PR is needed to (later) fix #48656 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I'd say so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably in the "Other" section and then the PR that fixes #48656 in the "Regression" section