Forward references in annotations with a signature changing decorator

[lucaswiman]

typing.get_type_hints uses the __wrapped__ attribute for finding the namespace of the original (undecorated) method was defined in. See here for the implementation in the standard library. This allows dereferencing of forward references, e.g.:

foo.py:

def foo(bar: "Bar", baz: str):
    pass
class Bar:
    pass

othermodule.py:

from foo import foo
import makefun, typing
@makefun.wraps(foo, remove_args=["baz"])
def decorated(*args, **kwargs):
    return foo(*args, **kwargs, baz="baz")

typing.get_type_hints(foo)  # fine
typing.get_type_hints(decorated)  # NameError

This then fails with:

$ PYTONPATH=.:$PYTHONPATH python ./mymodule.py
Traceback (most recent call last):
  File "/private/tmp/asdf/./mymodule.py", line 7, in <module>
    typing.get_type_hints(decorated)
  File "/Users/lucaswiman/.pyenv/versions/3.9.1/lib/python3.9/typing.py", line 1442, in get_type_hints
    value = _eval_type(value, globalns, localns)
  File "/Users/lucaswiman/.pyenv/versions/3.9.1/lib/python3.9/typing.py", line 277, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "/Users/lucaswiman/.pyenv/versions/3.9.1/lib/python3.9/typing.py", line 533, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
NameError: name 'Bar' is not defined

Things that did not work

1. Setting __wrapped__ to the original function. This gets rid of the NameError, but also changes the signature back to its unaltered form.
2. Setting __globals__. This is a readonly attribute. Also this might mess up execution of the decorated method.

Possible approach

If there were some way to define a method with the right signature and the right __globals__, then you could set the __wrapped__ attribute to that method and things would work correctly.

[lucaswiman]

Adding the following lines after the definition of seems to work:

import types
aux = types.FunctionType(
    decorated.__code__, foo.__globals__, name=decorated.__name__, argdefs=decorated.__defaults__, closure=foo.__closure__
    )
decorated.__wrapped__ = aux

(Loosely based off this Stackoverflow answer: https://stackoverflow.com/a/13503277)

Is that too convoluted? If you're amenable, I'd be happy to make a PR for this.

[lucaswiman]

PEP 362 gives the following logic for the return value of inspect.signature and related behavior:

The function implements the following algorithm:

• If the object is not callable - raise a TypeError
• If the object has a __signature__ attribute and if it is not None - return it
• If it has a __wrapped__ attribute, return signature(object.__wrapped__)
• [...]

It appears all that's required is when the signature changes, set the __signature__ attribute, then always set the __wrapped__ attribute to correctly handle forward references on decorated functions.

[smarie]

good catch @lucaswiman !

[smarie]

@lucaswiman : does this bug also happen if you use create_function (but not wraps) ? It is indeed key to decide if we should perform the change only for wraps or always in create_function.

From PEP362:

the user can manually cache a Signature by storing it in the __signature__ attribute.
It is better to reserve the __signature__ attribute for the cases when there is a need to explicitly set to a Signature object that is different from the actual one

So I would be in favor of scoping down the issue to the only situation(s) where it actually happens, and set the __signature__ attribute explicitly only there.

[lucaswiman]

Comment moved from the PR per your request, though I think this is more of an implementation detail than a decision about features:

The problem is that if you pass a string signature to wraps, then __signature__ ends up as a string as well. wraps knows whether it needs to set __signature__, so I don't think you can move that logic here. But, you don't have access to the compiled signature object in wraps, only in this method.

It seems like the code is just a bit awkwardly factored to handle this edge case, and this seemed like the least disruptive place to put this logic.

So I see two options:

1. Since it is always an error to set __signature__ to a string value, we could set attrs['__signature__'] = get_signature_from_string(attrs['__signature__'], evaldict)[1] when attrs[__signature__] is a string.
2. Delay setting both __wrapped__ and __signature__ until after the call to this function in wraps (or with_signature).

I guess (2) is more in line with what you're asking for. (1) seems a bit simpler, and I think it would never do the wrong thing. (I'm sort of guessing what the right behavior is here, since I don't have a good understanding of the use cases for specifying the signature as a string.)

[smarie]

Thanks again @lucaswiman ! Note: I eventually modified your contribution to perform get_signature_from_string inside wraps, for consistency. That way, create_function does not try to be intelligent about the __signature__ attribute, whether it is present or not. I added a test to make sure that the evaldict used would be the right one.

I'll now proceed to 1.15.0 release , thanks again !