Conversation
| return functools.partial(_define_aliases, alias_d) | ||
|
|
||
| def make_alias(name): # Enforce a closure over *name*. | ||
| def method(self, *args, **kwargs): |
There was a problem hiding this comment.
Just add a @functools.wraps(getattr(cls, name)) decorator here? inspect.signature will then get the signature from the wrappee. Note that you can even play with the assigned kwarg to functools.wraps if you don't want to update the name...
There was a problem hiding this comment.
Thanks for the hint. That would work too.
The getattr-alias is a bit slower than my explictly coded lambda (67ns vs. 26ns). I mean, it's not much in absolute values, but the aliases might be used a lot by users. Do we want the added speedup or should we stay with the slightly simpler but slower getattr implementiation.
There was a problem hiding this comment.
No strong opinion there; though eval() whill likely have a small extra startup cost as well?
There was a problem hiding this comment.
There are 60 properties. The package import time does not measureably change between the two approaches (200.8 +/- 6.0ms vs. 201.0 +/- 6.0ms).
Probably both approaches are fast enough in startup as well as runtime use to not make a difference. I will go back to decorating the original function to keep it simpler. If runtime performance is really super-critical, users should just use the methods functions, not the aliases.
lib/matplotlib/cbook/__init__.py
Outdated
| return method | ||
| def make_alias(method): | ||
| import inspect | ||
| args = str(inspect.signature(method))[1:-1] # remove parentheses |
There was a problem hiding this comment.
str(inspect.signature(functools.partial(method, None))) will bind the first argument for you and remove it from the signature more easily than the text wrangling below :) (plus it even works if someone decides to not call the first argument "self", hehe... for example it could just be a wrapper function itself being *args, **kwargs)
|
For future reference, this was the first approach, which was discarded in favor of simply decorating the alias with |
e5b62d9 to
1093f98
Compare
NelleV
left a comment
There was a problem hiding this comment.
This looks good to me.
(although about this whole functionality:
There should be one-- and preferably only one --obvious way to do it. 😭 )
|
Thanks @timhoffm |
PR Summary
Until now, all alias methods had the signature
*args, **kwargs. This is not very helpful as it's unclear which arguments are expected (well, usually none for getters and one for setters, but the alias method does not tell).This PR uses inspect to copy the signatures of the original functions to the alias.
Example setter
before:
now:
Example getter
before:
now:
