8000 functools.update_wrapper copies all __dict__ attributes to the wrapper by default, which can be surprising when used on callable instances · Issue #105933 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

functools.update_wrapper copies all __dict__ attributes to the wrapper by default, which can be surprising when used on callable instances #105933

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.

Sign up for GitHub

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

Jump to bottom
Open
purpledog opened this issue Jun 20, 2023 · 3 comments
Open

functools.update_wrapper copies all __dict__ attributes to the wrapper by default, which can be surprising when used on callable instances #105933

purpledog opened this issue Jun 20, 2023 · 3 comments
Labels
docs Documentation in the Doc dir

Comments

@purpledog
Copy link
purpledog commented Jun 20, 2023 •
edited by carljm
Loading

Bug report

functools.update_wrapper silently remove dataclass function wrapper copies all attributes from wrapped callable to wrapper, which can lead to surprising behavior. In this case the fun attribute is copied, totally changing the behavior of the code:

Repro:

import dataclasses
import functools
from typing import Any, Callable

@dataclasses.dataclass()
class PlusOne:
  fun: Callable[..., int]

  def __post_init__(self):
    functools.update_wrapper(self, self.fun)
  
  def __call__(self, *args, **kwargs):
    return self.fun(*args, **kwargs) + 1

def identity(x):
  return x

print(PlusOne(identity)(0))  # 1
print(PlusOne(PlusOne(identity))(0))  # 1

Removing __post_init__ fixes the issue.

Your environment

Linux, 3.10.11
@purpledog purpledog added the type-bug An unexpected behavior, bug, or error label Jun 20, 2023
@sunmy2019
Copy link
Member

Don't use functools.update_wrapper then.

There is a piece of code updating PlusOne(identity).fun:

def update_wrapper(wrapper,
                   wrapped,
                   assigned = WRAPPER_ASSIGNMENTS,
                   updated = WRAPPER_UPDATES):
    for attr in assigned:
        try:
            value = getattr(wrapped, attr)
        except AttributeError:
            pass
        else:
            setattr(wrapper, attr, value)
    for attr in updated:
        getattr(wrapper, attr).update(getattr(wrapped, attr, {}))
    # Issue #17482: set __wrapped__ last so we don't inadvertently copy it
    # from the wrapped function when updating __dict__
    wrapper.__wrapped__ = wrapped
    # Return the wrapper so this can be used as a decorator via partial()
    return wrapper
    for attr in updated:
        getattr(wrapper, attr).update(getattr(wrapped, attr, {}))

@sunmy2019 sunmy2019 added pending The issue will be closed if no feedback is provided and removed type-bug An unexpected behavior, bug, or error labels Jun 21, 2023
@carljm
Copy link
Member
carljm commented Jun 22, 2023

The culprit here is the __dict__ update; when the wrapped is a PlusOne instance, this results in copying the wrapped fun attribute to the wrapper.

This does mean that the default behavior of functools.update_wrapper can be a bit of a foot-gun when used with callable instances rather than normal functions, since all attributes of the wrapped callable will be copied to the wrapper callable by default.

This is easy to work around; just use functools.update_wrapper(self, self.fun, updated=[]) to avoid the default updating of __dict__.

This behavior of functools.update_wrapper is clearly documented and longstanding, therefore I don't think it's a bug, nor do I think it will be changed.

I do think the update_wrapper documentation could be clearer about this risk, however, especially since it specifically notes that update_wrapper is usable with callable instances, not just functions. I would support a documentation update briefly mentioning this risk, in the paragraph that begins "update_wrapper() may be used with callables other than functions." Some wording like "The default behavior of updating all __dict__ contents may copy more attributes than desired when used with callable instances; passing updated=[] will avoid this."

@carljm carljm changed the title functools.update_wrapper silently remove dataclass function wrapper functools.update_wrapper copies all __dict__ attributes to the wrapper by default, which can be surprising when used on callable instances Jun 22, 2023
@carljm
Copy link
Member
carljm commented Jun 22, 2023

I updated the wording of the OP to clarify the actual issue.

@carljm carljm added docs Documentation in the Doc dir and removed pending The issue will be closed if no feedback is provided labels Jun 22, 2023
@parthea parthea mentioned this issue Jun 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
docs Documentation in the Doc dir
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@carljm @purpledog @sunmy2019
0