Skip to content

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Nov 20, 2023

Following the discussion of #108902, I documented that getargs is deprecated (since it is not documented anywhere, I just modified its docstring) and that inspect.signature(types.FunctionType(co, {})) is the modern alternative. I think that it is safe to remove this function in two versions, because it does not work anyway (pos-only, kw-only params are incorrect).

I also don't think that modernizing getargvalues is valuable, because it will also be deprecated in 3.13 and hopefully removed in 3.15 as well.

CC @AlexWaygood @merwok @vstinner who reviewed the first PR.

Comment on lines +1506 to +1509
import warnings
with warnings.catch_warnings():
warnings.simplefilter('ignore', category=DeprecationWarning)
args, varargs, varkw = getargs(frame.f_code)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should follow our own advice and use inspect.signature(types.FunctionType(frame.f_code, {})) here? :)

Copy link
Member Author

@sobolevn sobolevn Nov 22, 2023

Choose a reason for hiding this comment

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

I don't think so. Right now getargs does some ugly things:

return Arguments(args + kwonlyargs, varargs, varkw)
It combines pos_or_kw_args together with kw_only_args, but ignores pos_only_args.

I think that we can just keep it as-is and then remove all of them together in 3.15 (because getargvalues will also be deprecated and removed at the same time, PR is just not ready yet).

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I see. Maybe we should do the getargvalues() deprecation first, though, in that case? It makes me a little uncomfortable adding this deprecation warning now, if we're not able to make the mandated change in our own code yet.

(I'm fine suppressing DeprecationWarnings in a function that is itself deprecated, but getargvalues() isn't, yet. And I know you plan to work on deprecating it immediately after this PR, but if I had a pound for every time somebody promised me they'd work on something the next day, and then discovered it was harder than they expected, I'd be a rich man :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I can open a PR about getargvalues() first 👍

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! As discussed in https://github.com/python/cpython/pull/112279/files#r1402002908, however, let's deprecate getargvalues() first 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants