Setting attributes on a decorated method mutates the original method

[lucaswiman]

When you decorate a method, you may want to set some metadata on the decorated object. However, doing so also mutates the original method, which is clearly undesirable.

In particular, you might want to set different metadata for separate decorations of the same method.

How to reproduce:

>>> import wrapt
>>>
>>> @wrapt.decorator
... def deco(func, instance, args, kwargs):
...     return func(*args, **kwargs)
...
>>>
>>> def foo():
...     return 1
...
>>> bar = deco(foo)
>>> assert not hasattr(foo, "myattr")
>>> bar.myattr = 1
>>> assert not hasattr(foo, "myattr")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError

[GrahamDumpleton]

See:

• https://wrapt.readthedocs.io/en/latest/wrappers.html?highlight=_self_#proxy-object-attributes

In other words use the convention:

bar._self_myattr = 1

If you want the attribute on the wrapper and not the wrapped object.

[lucaswiman]

Proxying __setattr__ is good for object proxies, though it wasn't obvious from reading the docs that decorators return object proxies. Would you be willing to accept a documentation change warning that setting attributes on decorated methods may mutate the original method?

Separately, the functionality still feels off... I don't see any use cases where you would want to have wrapt.decorator's return value set attributes on the original method. It seems like an implementation detail leaking through. You want the decorated method to have the same attributes as the original method, and that happens to be implemented using an object proxy. But then you pull along the __setattr__ behavior, which in my opinion does not make sense for the return values of decorators. Would disabling the custom __setattr__ functionality for decorators break anything that you're aware of?

[GrahamDumpleton]

You can't know how people use stuff and on that basis I would suspect that changing the behaviour would break stuff.

Imagine a case where there are multiple levels of decorators and a middle decorator checks for an attribute on itself which would be set externally on the complete wrapped function.

Someone comes along and adds a new decorator on top, or does monkey patching. The original external code still just sets the attribute on the complete wrapped function. If that wasn't propagated down through all layers to the original wrapped object, then that middle decorator would not then see its attribute being set anymore and behaviour could be broken.

[lucaswiman]

There is a separate bug when using adapter_factory to alter the argspec of a method, where it does not set __annotations__ to match the argspec. I had been fixing that by just setting __annotations__ on the return value, but that also propagates to the decorated method. Here is a contrived example reproducing the problem:

>>> import wrapt, inspect
>>>
>>>
>>> def example(i: int) -> int:
...     pass
...
>>>
>>> argspec = inspect.getfullargspec(example)
>>>
>>> @wrapt.decorator(adapter=wrapt.adapter_factory(lambda _: argspec))
... def deco(wrapped, instance, args, kwargs):
...     [i] = args
...     wrapped()
...     return i + 1
...
>>>
>>> def foo() -> None:
...     print("called")
...
>>> bar = deco(foo)  # Does not set __annotations__ on bar.
>>>
>>> foo.__annotations__  # Still correct
{'return': None}
>>> # Now update the annotations on bar so that they are correct:
>>> bar.__annotations__ = argspec.annotations
>>> bar.__annotations__  # correct
{'return': <class 'int'>, 'i': <class 'int'>}
>>> foo.__annotations__  # wrong
{'return': <class 'int'>, 'i': <class 'int'>}

I don't see a workaround for that, though perhaps setting __annotations__ could be special-cased.

Update: Actually, I think I can just "copy" the function before decorating for my use case: https://stackoverflow.com/a/30714299 Update 2: python-makefun might also be useful if you run into these issues: https://smarie.github.io/python-makefun/

[GrahamDumpleton]

There are some caveats around using wrapt.adapter_factory with functions having annotations but I can't remember exact details right now. The code is:

                    # Check if the signature argument specification has
                    # annotations. If it does then we need to remember
                    # it but also drop it when attempting to manufacture
                    # a standin adapter function. This is necessary else
                    # it will try and look up any types referenced in
                    # the annotations in the empty namespace we use,
                    # which will fail.

                    annotations = {}

                    if not isinstance(adapter, string_types):
                        if len(adapter) == 7:
                            annotations = adapter[-1]
                            adapter = adapter[:-1]
                        adapter = formatargspec(*adapter)

                    exec_('def adapter{}: pass'.format(adapter), ns, ns)
                    adapter = ns['adapter']

                    # Override the annotations for the manufactured
                    # adapter function so they match the original
                    # adapter signature argument specification.

                    if annotations:
                        adapter.__annotations__ = annotations

The problem is that need to ignore annotations from the argspec when formatting the function adapter using exec. If you don't and type annotations use classes (rather than builtin types) they will not be known of in the scope were exec is done and will fail. As a result it tries to just copy them from the original adapter function supplied.

Further, some other weird stuff happens to try and handle introspection on the function when an adapter is used.

• https://github.com/GrahamDumpleton/wrapt/blob/develop/src/wrapt/decorators.py#L53-L168

It is entirely possible that this latter mess needs to take into consideration annotations.

Personally I haven't used annotations as generally had to also deal with Python 2.7 so never been any sort of expert on them or really tested them much.

Anyway, will try and wort through your examples and see if that mess of code needs to have special handling for annotations if doing updates or whether it got broken along the way, as it was necessary to change how stuff was done in Python 3.10, but that change would have got applied retrospectively back to about Python 3.7 as well if using older Python.

What version of Python are you using?

[lucaswiman]

There is of course no rush, and thanks very much for your detailed replies! For what it's worth, I think I have a workaround for my particular use case.

What version of Python are you using?

# python --version
Python 3.8.12

[GrahamDumpleton]

Do you happen to know if you are using the pure Python wrappers for wrapt, or the C extension version? There could be a difference in behaviour between the two.

[lucaswiman]

It looks like it's using the C extension version if I understand correctly:

>>> import wrapt._wrappers, wrapt.wrappers
>>> wrapt._wrappers.ObjectProxy is wrapt.wrappers.ObjectProxy
True

[lucaswiman]

I think the object proxy model that wrapt is using is incompatible with what I was trying to do. Short of a total redesign, I don't see a solution to the general "problem" (or feature) in the issue description, though you may want to consider special-casing __annotations__.

I am closing this issue because switching to python-makefun worked for my use case and is a straightforward workaround.