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

gh-108901: Deprecate inspect.getargs, slate it for removal in 3.15 #112279

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

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.

Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved
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 👍

Lib/inspect.py Outdated Show resolved Hide resolved
Lib/inspect.py Outdated Show resolved Hide resolved
sobolevn and others added 2 commits November 22, 2023 16:28
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
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