Skip to content

API: consider better decorator option #13875

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

Closed
chris-b1 opened this issue Aug 1, 2016 · 12 comments
Closed

API: consider better decorator option #13875

chris-b1 opened this issue Aug 1, 2016 · 12 comments
Labels
API Design Closing Candidate May be closeable, needs more eyeballs Docs Enhancement

Comments

@chris-b1
Copy link
Contributor

chris-b1 commented Aug 1, 2016

It's not an issue in py3.5, but in older versions this is annoying - because a decorator is used in the definition, the function signature on to_datetime is lost. This is the big one I've run into, but there may be some other places in the API it comes up.
image

I saw this blog today - could use one of the strategies there. It may make sense to vendor rather than adding another dependency?
https://hynek.me/articles/decorators/

@jreback
Copy link
Contributor

jreback commented Aug 1, 2016

it should propagate

I think it just needs wraps

@jreback
Copy link
Contributor

jreback commented Aug 2, 2016

actually @chris-b1 where is this from? ? in ipython and help work just fine. can you elaborate.

@jreback
Copy link
Contributor

jreback commented Aug 2, 2016

actually read that article. Ok that is nice.

@chris-b1
Copy link
Contributor Author

chris-b1 commented Aug 2, 2016

I'm assuming you're looking on python3? In python 2 the standard wraps doesn't propagate signatures.

In [1]: import functools

In [2]: def dec_stdlib(f):
   ...:     @functools.wraps(f)
   ...:     def inner(*args, **kwargs):
   ...:         f(*args, **kwargs)
   ...:     return inner

In [6]: import boltons.funcutils

In [7]: def dec_boltons(f):
   ...:     @boltons.funcutils.wraps(f)
   ...:     def inner(*args, **kwargs):
   ...:         f(*args, **kwargs)
   ...:     return inner

In [4]: @dec_stdlib
   ...: def f(a, b):
   ...:     return a + b

In [5]: f?
Signature: f(*args, **kwargs)
Docstring: <no docstring>
Type:      function


In [8]: @dec_boltons
   ...: def f(a, b):
   ...:     return a + b

In [9]: f?
Signature: f(a, b)
Type:      function

@jreback jreback added this to the Next Major Release milestone Aug 2, 2016
@jreback jreback added the Compat pandas objects compatability with Numpy or Python functions label Aug 2, 2016
@chris-b1
Copy link
Contributor Author

I attempted this using https://github.com/mahmoud/boltons/blob/master/boltons/funcutils.py - which has a wraps replacement, but the interaction of the magic broke a bunch of stuff. I'm sure it would be possible to work through it, but I'm letting this one sit - I guess it's good self-motivation to update things to python 3.

@jorisvandenbossche jorisvandenbossche added 2/3 Compat and removed Compat pandas objects compatability with Numpy or Python functions API Design labels Aug 11, 2016
@topper-123
Copy link
Contributor

I've looked into this a bit using python 3.7 and the getfullargspecs problem still stands in 3.7. I'll remove the 2/3 compat label, but let the issue stand open.

For the record, giving the drawbacks to wrapt and decorators.py, I like vendoring boltons.functools.wraps the best (assuming it can be pulled out painlessly - haven't checked that).
.

@jbrockmendel
Copy link
Member

jbrockmendel commented Jul 23, 2019

@chris-b1 since we're now py35+ is this closeable?

@chris-b1
Copy link
Contributor Author

I would think so! Although I am not following @topper-123 comment above on getfullargspecs so maybe some part of this that doesn't work?

@topper-123
Copy link
Contributor

My point was that inspect.getfullargspec still does not work 100 %. Quoting the the linked article:

It’s kind of cheating: the signatures are still broken. inspect.signature() is just smart enough to lie to you.

OTOH, if mypy accepts the "lie", all the better. Does anyone know? (PyCharm does not).

To demonstrate the difference between a decorated and non-decorated function:

>>> from inspect import getfullargspec
>>> from functools import wraps
>>> def my_func(a:int, b: int = 1) -> int:
    ...:     return a + b
>>> def deco(func):
...     @functools.wraps(func)
...     def inner_func(*args, **kwargs):
...         return func(*args, **kwargs)
...     return inner_func

>>> getfullargspec(my_func)
FullArgSpec(args=['a', 'b'], varargs=None, varkw=None, defaults=(1,), kwonlyargs=[], kwonlydefaults=None, annotations={'return': <class 'pandas.core.series.Series'>, 'a': <class 'pandas.core.series.Series'>, 'b': <class 'int'>})
>>> getfullargspec(deco(my_func))
FullArgSpec(args=[], varargs='args', varkw='kwargs', defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={'return': <class 'pandas.core.series.Series'>})

Notice the output is quite different.

inspect.signature gives the same output for both decorared and non-decorated, but that's not sufficient for at least PyCharm to treat a decorated function as the same as the non-decorated.

@jbrockmendel
Copy link
Member

@topper-123 now that we're on the verge of dropping py35, does that affect whether this is closeable and/or actionable?

@jbrockmendel
Copy link
Member

@WillAyd there was another decorator-related issue that just got closed. can this one go too?

@jbrockmendel jbrockmendel added the Closing Candidate May be closeable, needs more eyeballs label Sep 22, 2020
@WillAyd
Copy link
Member

WillAyd commented Sep 22, 2020

I think this works on master?

In [3]: inspect.getfullargspec(pd.to_datetime)                                                                                                                                                        
Out[3]: FullArgSpec(args=['arg', 'errors', 'dayfirst', 'yearfirst', 'utc', 'format', 'exact', 'unit', 'infer_datetime_format', 'origin', 'cache'], varargs=None, varkw=None, defaults=('raise', False, False, None, None, True, None, False, 'unix', True), kwonlyargs=[], kwonlydefaults=None, annotations={'return': typing.Union[pandas.core.indexes.datetimes.DatetimeIndex, ForwardRef('Series'), ~DatetimeScalar, ForwardRef('NaTType')], 'arg': typing.Union[~DatetimeScalar, typing.List, typing.Tuple, ~ArrayLike, ForwardRef('Series')], 'errors': <class 'str'>, 'dayfirst': <class 'bool'>, 'yearfirst': <class 'bool'>, 'utc': typing.Union[bool, NoneType], 'format': typing.Union[str, NoneType], 'exact': <class 'bool'>, 'unit': typing.Union[str, NoneType], 'infer_datetime_format': <class 'bool'>, 'cache': <class 'bool'>})

So sure let's close - can always reopen

@WillAyd WillAyd closed this as completed Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Closing Candidate May be closeable, needs more eyeballs Docs Enhancement
Projects
None yet
Development

No branches or pull requests

8 participants