-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
MNT: inspect.getfullargspec will be deprecated in py3.8 #14138
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
Conversation
Switch to using inspect.signature
lib/matplotlib/artist.py
Outdated
@@ -1270,7 +1270,7 @@ def _get_setters_and_targets(self): | |||
func = getattr(self.o, name) | |||
if not callable(func): | |||
continue | |||
nargs = len(inspect.getfullargspec(func).args) | |||
nargs = len(inspect.signature(func).parameters) |
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.
Technically, this is not the same because Signature.parameters
contains everything.
I think
len([p for p in inspect.signature(func).parameters.values()
if p.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD])
is equivalent, but haven't checked for cases with variable positional args or kwonly.
Some example code which may be useful to play around with:
>>> from pprint import pprint
>>> import inspect
>>> def f(a, b, c=0, **kwargs):
... pass
...
>>> inspect.getfullargspec(f)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw='kwargs', defaults=(0,), kwonlyargs=[], kwonlydefaults=None, annotations={})
>>> pprint(inspect.signature(f).parameters)
mappingproxy(OrderedDict([('a', <Parameter "a">),
('b', <Parameter "b">),
('c', <Parameter "c=0">),
('kwargs', <Parameter "**kwargs">)]))
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.
However, the only thing we use nargs
for is in the next line to detect if there if it less than 2 to detect if this is something that is named like a setter, but does not take enough arguments to actually be a setter.
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.
For the following
len(inspect.getfullargspec(func).args) < 2
is true but len(inspect.signature(func).parameters) < 2
is false:
Line2D.set_data(self, *args)
Axes.set_prop_cycle(self, *args, **kwargs)
Figure.set_constrained_layout_pads(self, **kwargs)
I did not check the full code path, but I assume they will be considered setters after the change - not sure if that's actually desired or not, but it's a change to be aware of.
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.
fair enough, will make the changes!
Thanks for pushing on this.
else: | ||
sig = inspect.signature(cls.__init__) | ||
argstr = ", ".join( | ||
f"{p.name}={p.default}" |
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.
Actually that's just str(p)
(which will print out as name=default
in presence of a default, isn't the stdlib wonderful? :p)
and the whole thing can be simplified down to
argstr = (str(inspect.signature(cls))[1:-1] # Strip parentheses.
or "None")
because style classes don't take other arguments, and inspect.signature(cls)
helpfully drops self
from the signature of __init__
...
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.
"if something seems like it is too hard, it probably is"
Looks like |
Superseeded by #14962. |
Switch to using inspect.signature
Still need to verify that the replacement is equivalent (which is why I marked it as draft), but pushing this out early so I don't forget I did this.