python / cpython

The Python programming language
https://www.python.org
Other
63.49k stars 30.41k forks source link

Type annotations lost when using wraps by default #85403

Open e508c9b1-f137-4a74-8873-03ddfcb92496 opened 4 years ago

e508c9b1-f137-4a74-8873-03ddfcb92496 commented 4 years ago
BPO 41231
Nosy @terryjreedy, @david-caro
PRs
  • python/cpython#21392
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['3.8', 'type-bug', 'library', '3.9', '3.10'] title = 'Type annotations lost when using wraps by default' updated_at = user = 'https://github.com/david-caro' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'David Caro' dependencies = [] files = [] hgrepos = [] issue_num = 41231 keywords = ['patch'] message_count = 6.0 messages = ['373233', '373238', '373258', '373299', '373303', '373497'] nosy_count = 3.0 nosy_names = ['terry.reedy', 'Terry Davis', 'David Caro'] pr_nums = ['21392'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue41231' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

    e508c9b1-f137-4a74-8873-03ddfcb92496 commented 4 years ago

    In version 3.2, bpo-8814 introduced copying the __annotations__ property from the wrapped function to the wrapper by default.

    That would be the desired behavior when your wrapper function has the same signature than the function it wraps, but in some cases (for example, with the contextlib.asynccontextmanager function) the return value is different, and then the __annotations__ property will have invalid information:

    In [2]: from contextlib import asynccontextmanager

    In [3]: @asynccontextmanager ...: async def mytest() -> int: ...: return 1 ...:

    In [4]: mytest.__annotations__
    Out[4]: {'return': int}

    I propose changing the behavior of wraps, to only assign the __annotations by default if there's no __annotations already in the wrapper function, that would fit most default cases, but would allow to preserve the __annotations of the wrapper function when the types are explicitly specified, allowing now to change the contextlib.asynccontextmanager function with the proper types (returning now an AsyncContextManager) and keep the __annotation valid.

    I'll try to get a POC and attach to the issue, but please comment with your ideas too.

    4f2e3846-682e-4d98-8367-e267de3ba0d6 commented 4 years ago

    I don't understand this use-case, but would it make sense to ChainMap the wrapper's annotations on top of the wrapped annotations?

    e508c9b1-f137-4a74-8873-03ddfcb92496 commented 4 years ago

    Hi Terry,

    That would not work in this case, as I'd want to override all annotations with the wrapper function ones if there's any, instead of merging them.

    The specific use case, is a type checker (part of TestSlide testing framework), to verify that if there's any type annotations, the parameters mocked and passed to it are the expected types.

    For example, the contextmanager decorator returns an actual ContextManager, wrapping whatever the wrapped function returned, so if the wrapped function annotations prevail, then there's no way if verifying that the returned type is correct.

    Thanks for the ChainMap pointer though, I'll use it for sure somewhere else.

    e508c9b1-f137-4a74-8873-03ddfcb92496 commented 4 years ago

    As a note, mypy does not tpyecheck the wrapper functions, probably because it would not be possible with the current code (as the typing hints get lost):

    https://mypy.readthedocs.io/en/latest/generics.html?highlight=wrapper#declaring-decorators

    e508c9b1-f137-4a74-8873-03ddfcb92496 commented 4 years ago

    Elaborating on the last message, given the following code:

      1 #!/usr/bin/env python3
      2 
      3 from functools import wraps
      4 
      5 
      6 def return_string(wrapped):
      7     @wraps(wrapped)
      8     def wrapper(an_int: int) -> str:
      9         return str(wrapped(an_int))
     10 
     11     return wrapper
     12 
     13 
     14 @return_string
     15 def identity(an_int: int) -> int:
     16     return an_int
     17 
     18 def print_bool(a_bool: bool) -> None:
     19     print(a_bool)
     20 
     21 def identity_nonwrapped(an_int: int) -> int:
     22     return an_int
     23 
     24 
     25 print_bool(a_bool=identity(7))
     26 print_bool(a_bool=identity_nonwrapped(7))

    mypy will complain only on the last line, being unable to check properly the line 25.

    I'll investigate a bit more on why mypy skips that.

    terryjreedy commented 4 years ago

    Only 3.8+ for bug fixes.

    david-caro commented 2 years ago

    This is fixed by #21392

    david-caro commented 2 years ago

    It will need rebasing though :)