On 08/07 10:49, Joao S. O. Bueno wrote:
> "3" would "break stuff around".
> I'd be glad if "2" worked - but that have to be well documented and
> easy to see.
I've update the PR with improved documentation and comments to make it easier
to see.
At this point I'd appreciate if I get some reviews to improve the patch,
thanks!
>
> "1" is basically a workaround the current behavior, and people who
> care for it will have to continue doing that until Python 3.10 is
> widespread - but ultimately it basically requires that everyone doing that
> re-implements whatever logic you have in that PR for "2" (thus defeating
> the very purpose of "@wraps" - I'd rather rebind wrapper.__name__ and
> __wrapped__ manually)
>
> TL;DR: I agree that "2" is the way to go.
>
>
> On Fri, 7 Aug 2020 at 09:14, David Caro wrote:
>
> >
> > Hi!
> >
> > Some time ago I opened a bug and sent a patch, but I realize that I
> > should have started a chat here first, better late than never.
> >
> > The default behavior of the `functools.wraps` decorator, is to copy over
> > the `__annotations__` from the wrapped function to the wrapper function.
> >
> > This is ok if the wrapper function has the same signature that the
> > wrapped one (that I guess was the main goal behind it, just a simple
> > wrapper).
> >
> > The issue comes when the wrapper function has a different signature than
> > the wrapped, for example, the `contextlib.contextmanager` decorator,
> > changes the return value, returning a `_GeneratorContextManager`:
> >
> > ```
> > def contextmanager(func):
> > """...
> > """
> > @wraps(func)
> > def helper(*args, **kwds):
> > return _GeneratorContextManager(func, args, kwds)
> > return helper
> > ```
> >
> > but as it uses `wraps`, the `__annotations__` will be copied over, and
> > the new function will have an incorrect return type there.
> >
> >
> > In order to improve this, I have some proposals:
> >
> > 1. Keep the default behavior of `wraps`, but change the usage around the
> > library to not copy over the `__annotations__` in places (like
> > `contextmanager`) where the types change.
> >
> > Advantages are that `wraps` keeps being backwards compatible, though the
> > change in `contextmanager` might force some people to "fix" their
> > annotations, I would consider that a "bugfix", more than a behavior change.
> >
> > The disadvantage is that you have to know that `wraps` will overwrite
> > the wrapped function annotations by default, and I think that it's
> > broadly used when creating decorators that have different signature/types
> > than
> > the wrapped function, so people will have to explicitly change their
> > code.
> >
> >
> > 2. Change `wraps` so if there's any type of annotation in the wrapper
> > function, will not overwrite it.
> > This is what I did in the PR (though I'm not convinced).
> >
> > Advantages are that only people that took the time to annotate the
> > wrapper function will see the change, and that will be the change they
> > expect (that's my guess though).
> > It's a bit smart with it, so if you don't specify a return type, will
> > get it from the wrapped function, or if you don't specify types for the
> > arguments will get them from the wrapped function, filling only the gap
> > that was not specified.
> > For everyone else, `wraps` is backwards compatible.
> >
> > Disadvantages, it's a bit more convoluted as it requires some logic to
> > detect what annotations are defined in the wrapper and which ones in the
> > wrapped and merge them.
> >
> >
> > 3. Change the default behavior of `wraps` and don't overwrite the
> > `__annotations__` property. This is non-backwards compatible, but imo
> > the simplest.
> >
> > Advantages are that it's very simple, and you'll end up with valid,
> > straight-forward `__annotations__` in the wrapped function (that is,
> > whatever you declare when writing the wrapped function).
> >
> > Disadvantages, if the goal of the `wraps` decorator was as a helper when
> > wrapping functions in a simple manner (not changing the signature), then
> > this becomes a bit more inconvenient, as it will not copy the
> > `__annotations__` as it was doing by default. It also changes the
> > current default behavior, so it will potentially affect everyone that
> > uses `wraps` currently.
> >
> >
> > Ideas?
> >
> >
> > Thanks!
> >
> > --
> > David Caro
> > ___
> > Python-ideas mailing list -- python-ideas@python.org
> > To unsubscribe send an email to python-ideas-le...@python.org
> > https://mail.python.org/mailman3/lists/python-ideas.python.org/
> > Message archived at
> > https://mail.python.org/archives/list/python-ideas@python.org/message/LFF3EXFPZG3FMXY4AXORU6SXXKBPSZOY/
> > Code of Conduct: http://python.org/psf/codeofconduct/
> >
--
David Caro
___
Python-ideas mailing list -- python-ideas@python.org
To unsubscribe send an email to
