[issue41232] Python `functools.wraps` doesn't deal with defaults correctly
Thor Whalen added the comment: On the surface, seems like a fair design to me: Back-compatible yet solves this misalignment that bugged me (literally). It would also force the documentation of `functools.wraps` to mention this "trap", through describing the `signature_changed` flag. As for the `__wrapped_with_changed_signature__`; yes, it's terrible. But I have no better. At least, it's very descriptive! On Sat, May 1, 2021 at 9:52 PM Jelle Zijlstra wrote: > > Jelle Zijlstra added the comment: > > We could add a new argument to `@functools.wraps()` to differentiate > between a wrapper with the same signature and one with a different > signature. > > Here's a possible design: > * functools.wraps adds a new keyword-only argument signature_changed. It > defaults to False for backward compatibility. > * If signature_changed is True: > * __annotations__ are not copied > * __wrapped__ is not set on the wrapping function. Instead, we set a new > attribute __wrapped_with_changed_signature__ (that's a pretty terrible > name, open to suggestions). This will make inspect.signature not look at > the wrapped function. > > -- > > ___ > Python tracker > <https://bugs.python.org/issue41232> > ___ > -- ___ Python tracker <https://bugs.python.org/issue41232> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40374] collections.abc docs table: Mapping missing __reversed__
New submission from Thor Whalen : `Mapping.__reversed__` exists, but is not listed in the table: https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes https://github.com/python/cpython/blob/master/Doc/library/collections.abc.rst -- assignee: docs@python components: Documentation messages: 367127 nosy: Thor Whalen2, docs@python, rhettinger, stutzbach priority: normal pull_requests: 19001 severity: normal status: open title: collections.abc docs table: Mapping missing __reversed__ versions: Python 3.8 ___ Python tracker <https://bugs.python.org/issue40374> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40374] collections.abc docs table: Mapping missing __reversed__
Thor Whalen added the comment:
Dennis,
Thanks for the (very complete) explanation. I induced to err because of the
following confusing facts:
```
from collections.abc import Mapping, Sequence
import sys
if sys.version_info <= (2, 7):
assert '__reversed__' not in dir(Mapping)
elif sys.version_info >= (3, 7):
assert not isinstance(Mapping, Sequence)
assert '__reversed__' in dir(Mapping)
assert list(reversed(dict.fromkeys('abc'))) == ['c', 'b', 'a']
# + no mention of Mapping.__reversed__ in the docs
```
Indeed, I didn't look at the code. Thanks for that.
On Thu, Apr 23, 2020 at 5:28 PM Dennis Sweeney
wrote:
>
> Dennis Sweeney added the comment:
>
> > `Mapping.__reversed__` exists
>
> While ``'__reversed__' in dir(Mapping)`` is true, that unfortunately does
> not mean that it is a real callable method:
>
>
> from collections.abc import Mapping
> class Map(Mapping):
> def __getitem__(self, x):
> if x == 42:
> return 17
> raise KeyError
> def __len__(self, x):
> return 1
> def __iter__(self):
> yield 42
>
> >>> m = Map()
> >>> reversed(m)
> Traceback (most recent call last):
> ...
> TypeError: 'Map' object is not reversible
>
> In the code for Mapping, ``__reversed__`` is explicitly defined as None
> [1] so that calling ``reversed(custom_mapping)`` doesn't accidentally fall
> back on the sequence protocol, which would mean starting at
> len(custom_mapping)-1 and calling __getitem__ on each index [2], which is
> certainly not desirable for arbitrary mappings.
>
> I don't think there is a reasonable way, given arbitrary implementations
> of __len__, __iter__, and __getitem__, to have a mixin reversed iterator.
> If someone wants their mapping to have a __reversed__ method, they should
> define it themselves.
>
> [1]
> https://github.com/python/cpython/blob/master/Lib/_collections_abc.py#L707
> [2]
> https://docs.python.org/3/reference/datamodel.html?highlight=__reversed__#object.__reversed__
>
> --
> nosy: +Dennis Sweeney
>
> ___
> Python tracker
> <https://bugs.python.org/issue40374>
> ___
>
--
nosy: +Thor Whalen
___
Python tracker
<https://bugs.python.org/issue40374>
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41232] Python `functools.wraps` doesn't deal with defaults correctly
New submission from Thor Whalen : # 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? -- messages: 373254 nosy: Thor Whalen2 priority: normal severity: normal status: open title: Python `functools.wraps` doesn't deal with defaults correctly type: enhancement versions: Python 3.8 ___ Python tracker <https://bugs.python.org/issue41232> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41232] Python `functools.wraps` doesn't deal with defaults correctly
Thor Whalen added the comment: 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 -- keywords: +patch message_count: 1.0 -> 2.0 pull_requests: +20523 stage: -> patch review pull_request: https://github.com/python/cpython/pull/21379 ___ Python tracker <https://bugs.python.org/issue41232> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41232] Python `functools.wraps` doesn't deal with defaults correctly
Thor Whalen added the comment: 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 -- ___ Python tracker <https://bugs.python.org/issue41232> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41232] Python `functools.wraps` doesn't deal with defaults correctly
Thor Whalen added the comment: 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 wrote: > > Terry J. Reedy added the comment: > > Is this actually a bugfix? > > -- > nosy: +terry.reedy > versions: +Python 3.10 -Python 3.8 > > ___ > Python tracker > <https://bugs.python.org/issue41232> > ___ > -- nosy: +Thor Whalen ___ Python tracker <https://bugs.python.org/issue41232> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue41232] Python `functools.wraps` doesn't deal with defaults correctly
Thor Whalen added the comment: 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 wrote: > > Dominic Davis-Foster 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 > <https://bugs.python.org/issue41232> > ___ > -- ___ Python tracker <https://bugs.python.org/issue41232> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
