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/

Reply via email to