classification
Title: Python `functools.wraps` doesn't deal with defaults correctly
Type: enhancement Stage: patch review
Components: Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Thor Whalen, Thor Whalen2, domdfcoding, donovick, ncoghlan, terry.reedy
Priority: normal Keywords: patch

Created on 2020-07-07 22:00 by Thor Whalen2, last changed 2020-08-07 03:01 by donovick.

Pull Requests
URL Status Linked Edit
PR 21379 open Thor Whalen2, 2020-07-07 22:10
Messages (8)
msg373254 - (view) Author: Thor Whalen (Thor Whalen2) Date: 2020-07-07 22:00
# PROBLEM

When using `functools.wraps`, the signature claims one set of defaults, but the (wrapped) function uses the original (wrappee) defaults.

Why might that be the desirable default?

# PROPOSED SOLUTION
Adding '__defaults__',  '__kwdefaults__' to `WRAPPER_ASSIGNMENTS` so that actual defaults can be consistent with signature defaults.

# Code Demo

```python
from functools import wraps
from inspect import signature

def g(a: float, b=10):
    return a * b

def f(a: int, b=1):
    return a * b

assert f(3) == 3

f = wraps(g)(f)

assert str(signature(f)) == '(a: float, b=10)'  # signature says that b defaults to 10
assert f.__defaults__ == (1,)  # ... but it's a lie!
assert f(3) == 3 != g(3) == 30  # ... and one that can lead to problems!
```

Why is this so? Because `functools.wraps` updates the `__signature__` (including annotations and defaults), `__annotations__`, but not `__defaults__`, which python apparently looks to in order to assign defaults.

One solution is to politely ask wraps to include these defaults.

```python
from functools import wraps, WRAPPER_ASSIGNMENTS, partial

my_wraps = partial(wraps, assigned=(list(WRAPPER_ASSIGNMENTS) + ['__defaults__', '__kwdefaults__']))

def g(a: float, b=10):
    return a * b

def f(a: int, b=1):
    return a * b

assert f(3) == 3

f = my_wraps(g)(f)

assert f(3) == 30 == g(3)
assert f.__defaults__ == (10,)  # ... because now got g defaults!
```

Wouldn't it be better to get this out of the box?

When would I want the defaults that are actually used be different than those mentioned in the signature?

<!--
Thanks for your contribution!
Please read this comment in its entirety. It's quite important.

# Pull Request title

It should be in the following format:

```
bpo-NNNN: Summary of the changes made
```

Where: bpo-NNNN refers to the issue number in the https://bugs.python.org.

Most PRs will require an issue number. Trivial changes, like fixing a typo, do not need an issue.

# Backport Pull Request title

If this is a backport PR (PR made against branches other than `master`),
please ensure that the PR title is in the following format:

```
[X.Y] <title from the original PR> (GH-NNNN)
```

Where: [X.Y] is the branch name, e.g. [3.6].

GH-NNNN refers to the PR number from `master`.

-->
msg373255 - (view) Author: Thor Whalen (Thor Whalen2) Date: 2020-07-07 22:10
Posted to stackoverflow to gather opinions about the issue: https://stackoverflow.com/questions/62782230/python-functools-wraps-doesnt-deal-with-defaults-correctly

Also made GitHub PR: https://github.com/python/cpython/pull/21379
msg373256 - (view) Author: Thor Whalen (Thor Whalen2) Date: 2020-07-07 22:18
Further, note that even with the additional '__defaults__', and '__kwdefaults__', `functools.wraps` breaks when keyword only arguments involved:

```
from functools import wraps, WRAPPER_ASSIGNMENTS, partial

# First, I need to add `__defaults__` and `__kwdefaults__` to wraps, because they don't come for free...
my_wraps = partial(wraps, assigned=(list(WRAPPER_ASSIGNMENTS) + ['__defaults__', '__kwdefaults__']))

def g(a: float, b=10):
    return a * b

def f(a: int,  *, b=1):
    return a * b

# all is well (for now)...
assert f(3) == 3
assert g(3) == 30
```

This:
```
my_wraps(g)(f)(3)
```
raises TypeError (missing required positional argument 'b'), expected

Note that `wraps(g)(f)(3)` doesn't throw a TypeError, but the output is not consistent with the signature (inspect.signature(wraps(g)(f)) is (a: float, b=10), so 3 should be multiplied by 10). This is because __defaults__ wasn't updated. See for example, that third-party from boltons.funcutils import wraps works as expected. And so do (the very popular) wrapt and decorator packages. Boltons works for wraps(f)(g), but not wraps(g)(f) in my example. 

See: https://stackoverflow.com/questions/62782709/pythons-functools-wraps-breaks-when-keyword-only-arguments-involved
msg373499 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-07-11 01:58
Is this actually a bugfix?
msg374818 - (view) Author: Thor Whalen (Thor Whalen) * Date: 2020-08-04 14:59
Hi Terry, sorry for the later reply.

Is this a bugfix? Well, I'm not sure what you would call a bug. Can't one
always redefine a bug to be a feature, and visa versa?

I would definitely say that the behavior (seeing one default in the
signature, but a different one actually taking effect) is probably not a
good one -- as this could lead to very hard to find... bugs. It seems in
fact that third party "fix your decorators" packages such as `wrapt` and
`boltons.funcutils` agree, since their implementation of `wraps` doesn't
have this... "misaligned-by-default feature" that `functools.wraps` does.

Unless I'm missing something, my guess of why `functools.wraps` doesn't
include what I put in my pull request is that it breaks some tests. But I
had a look at the failing test and it seems that it is the test that is
"wrong" (i.e. tests for a behavior that really shouldn't be the default).

See comment:
https://github.com/python/cpython/pull/21379#issuecomment-655661983

The question is: Is there a lot of code out there that depends on this
misaligned behavior. My guess is not.

On Fri, Jul 10, 2020 at 9:58 PM Terry J. Reedy <report@bugs.python.org>
wrote:

>
> Terry J. Reedy <tjreedy@udel.edu> added the comment:
>
> Is this actually a bugfix?
>
> ----------
> nosy: +terry.reedy
> versions: +Python 3.10 -Python 3.8
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue41232>
> _______________________________________
>
msg374879 - (view) Author: Dominic Davis-Foster (domdfcoding) * Date: 2020-08-05 12:51
To me your examples seem like a misuse of functools.wraps. IIUC its purpose is to make a wrapper that accepts solely *args and **kwargs appear to be the wrapped function; it doesn't seem to be intended to be used when the wrapper takes arguments of different types or that have different default values. 

I can't think of a situation where you'd use wraps with a decorator that doesn't take just *args and **kwargs. That's not to say there aren't occasions where you'd want to to that, just that wraps isn't the right tool.
msg374881 - (view) Author: Thor Whalen (Thor Whalen) * Date: 2020-08-05 13:36
You are the guardians of the great python, so we can leave it at that if
you want. That said for posterity, I'll offer a defense.

The same "the tools does what it does, and if you need something else, use
another tool" argument could have been applied to vote against adding
`__annotations__` etc. back when it was lacking.
If there were clear use cases for having signature defaults be different
from actual defaults, I'd also agree with the argument.
If it were a complicated fix, I'd also agree with you.

But it's a simple fix that would help avoiding unintended misalignments.

I may be missing something, but the only positive reasons I see for keeping
it the way it is are: (1) backcompatibility safety, and (2) not having to
change the (in my opinion incorrect) tests.

If it is kept as such, I think a documentation warning would go a long way
in making users avoid the trap I myself fell into.

On Wed, Aug 5, 2020 at 8:51 AM Dominic Davis-Foster <report@bugs.python.org>
wrote:

>
> Dominic Davis-Foster <dominic@davis-foster.co.uk> added the comment:
>
> To me your examples seem like a misuse of functools.wraps. IIUC its
> purpose is to make a wrapper that accepts solely *args and **kwargs appear
> to be the wrapped function; it doesn't seem to be intended to be used when
> the wrapper takes arguments of different types or that have different
> default values.
>
> I can't think of a situation where you'd use wraps with a decorator that
> doesn't take just *args and **kwargs. That's not to say there aren't
> occasions where you'd want to to that, just that wraps isn't the right tool.
>
> ----------
> nosy: +domdfcoding
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue41232>
> _______________________________________
>
msg374900 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-08-05 21:22
Thor, in the future, when you reply by email, snip off the messages you are replying to.  When you your message is added to the webpage below the earlier message, the copy become extraneous noise.  Quoting the hidden boilerplate on PRs is also useless.

For the purpose of this tracker, a bug is a discrepancy between the code and the docs.  Since I have hardly used wraps nor read it docs in detail, I have no particular opinion.
History
Date User Action Args
2020-08-07 03:01:38donovicksetnosy: + donovick
2020-08-05 21:22:55terry.reedysetmessages: + msg374900
2020-08-05 13:36:45Thor Whalensetmessages: + msg374881
2020-08-05 12:51:16domdfcodingsetnosy: + domdfcoding
messages: + msg374879
2020-08-04 14:59:30Thor Whalensetnosy: + Thor Whalen
messages: + msg374818
2020-07-11 01:58:22terry.reedysetnosy: + terry.reedy

messages: + msg373499
versions: + Python 3.10, - Python 3.8
2020-07-08 06:42:47rhettingersetassignee: ncoghlan

nosy: + ncoghlan
2020-07-07 22:18:12Thor Whalen2setmessages: + msg373256
2020-07-07 22:10:41Thor Whalen2setkeywords: + patch

stage: patch review
messages: + msg373255
pull_requests: + pull_request20523
2020-07-07 22:00:22Thor Whalen2create