Skip to content

Generic decorators don't work with generic functions #1317

Closed

Description

@AlexOnAPeak

UPDATE: Although the original example is now special-cased in a plugin, the underlying problem is not solved, see #1317 (comment)

The following:

from typing import *
from contextlib import *

T = TypeVar("T")

@contextmanager
def foo(x):
  # type: (T) -> Iterator[T]
  yield x

y = 1
with foo(2) as f:
  y = f

gives this error:

error: Incompatible types in assignment (expression has type "T", variable has type "int")

It should recognize the input as an int which would lead to the output being an int as well, not a T. This is probably due to the way the contextmanager stub is defined in typeshed.

Activity

  1. gvanrossum commented on Apr 4, 2016

    @gvanrossum
    Member

    Yeah, contextmanager seems to bring out the worst in mypy, because it's a decorator and wraps a generator. :-( Note this comment in contextlib.pyi:

    # TODO this doesn't capture the relationship that the returned function's args are the same as func's.
    def contextmanager(func: Callable[..., Iterator[_T]]) -> Callable[..., ContextManager[_T]]: ...
    

    Fixing that will probably require some way to describe the relationship between the args of the Callable used to describe contextmanager's 'func' argument and the args of the Callable describing its return type. (Note that the return type of the two Callables are different.)

  2. JukkaL commented on Apr 5, 2016

    @JukkaL
    Collaborator

    Variadic type variables (python/typing#193) would be useful here, assuming that they could preserve all the details of function argument lists such as keyword argument names. They aren't in PEP 484 and mypy doesn't support them, so they won't help us right now.

    Another way to go about this would be to support type checker plugins that can special case the checking of certain library functions. Mypy doesn't support them either, so the current signature is about the best we can do.

  3. added this to the milestone on Apr 7, 2016
  4. rwbarton commented on Apr 8, 2016

    @rwbarton
    Contributor

    Variadic type variables would certainly be the way to go here, but there's actually a latent mypy bug here as well, which you can expose by changing the stub type of contextlib.contextmanager to the type that should make this program type check, namely

    def contextmanager(func: Callable[[_T], Iterator[_T]]) -> Callable[[_T], ContextManager[_T]]: ...
    

    Recall that

    @contextmanager
    def foo(x):
      # type: (T) -> Iterator[T]
      yield x
    

    really means something like

    def foo_(x):
      # type: (T) -> Iterator[T]
      yield x
    foo = contextmanager(foo_)
    

    foo_ is a generic function with a type variable T, and in the call to the generic function contextmanager we determine that _T = T. But foo does not end up being a generic function in mypy's internal representation. It is just a function whose type involves a type variable T that is not bound anywhere. So the call foo(2) then already fails to type check, since the type of 2 is int and not T.

    With the current type of contextmanager, foo gets the type Callable[..., ContextManager[T]], so foo(2) is accepted, but the result type ContextManager[T] still has a nonsensical reference to an unbound type variable.

    When inferring the type of the result of a call to a generic function, we need to take care when that type still contains type variables. If the result is a callable, then we should promote it to a generic callable ("generalization"). If the result is not callable, then mypy seems to already give an error message error: Need type annotation for variable, which is sensible.

    With such a change, we should almost be able to type check the original program. After all, the more specific type for contextmanager using variadic type variables should only serve to reject ill-typed programs, such as if you passed foo something other than an int.

  5. rwbarton commented on Apr 8, 2016

    @rwbarton
    Contributor

    ("almost be able to type check" because mypy actually refuses to infer a type for f, and AFAIK you cannot specify one (aside from writing a cast on foo(2)). See #1356.)

  6. rwbarton commented on Apr 8, 2016

    @rwbarton
    Contributor

    Actually, type generalization as I described above is technically unsound. (TODO: Insert realistic example here.) The result of the decorator could be a closure that refers to a mutable variable of a type that depends on the type variables of the closure. What matters for soundness of generalization is whether the value is syntactically defined as a function, not whether it has a function type.

    For the purposes of mypy it probably makes most sense not to worry about this point and generalize all values of function type.

  7. JukkaL commented on Apr 18, 2016

    @JukkaL
    Collaborator

    This may require fixing #603 first, which is not scheduled until 0.4.x. If that's the case, perhaps we should postpone this until 0.4.x?

  8. gvanrossum commented on Apr 18, 2016

    @gvanrossum
    Member

    Maybe, or we could move #603 up, or we could make both stretch goals for 0.4. It's really up to Reid.

  9. modified the milestones: , on May 5, 2016
  10. JukkaL commented on May 5, 2016

    @JukkaL
    Collaborator

    Moved to 0.4.x.

  11. removed this from the milestone on Mar 29, 2017
  12. 5 remaining items

  13. ilevkivskyi commented on Nov 5, 2017

    @ilevkivskyi
    Member

    @ethanhs The underlying issue is still not fixed however:

    def transform(f: Callable[..., T]) -> Callable[..., Iterator[T]]:
        ...
    
    def func(x: T) -> T:
        ...
    
    reveal_type(transform(func))  # Revealed type is 'def (*Any, **Any) -> typing.Iterator[T`-1]'

    The last line shows that transform(func) is not generic and has an unbound type variable in it. The same happens if one uses transform as a decorator. I think this works for contextmanager because we have a plugin.

    I therefore prefer to keep this issue open, since it has the discussion and a possible solution by Reid.

  14. changed the title [-]TypeVar fails to resolve in a context manager[/-] [+]Generic decorators don't work with generic functions[/+] on Nov 5, 2017
  15. ilevkivskyi commented on May 18, 2018

    @ilevkivskyi
    Member

    #1515 contains another important example of this bug in stdlib stubs.

  16. ilevkivskyi commented on Feb 11, 2019

    @ilevkivskyi
    Member

    #3193 shows another example in stdlib.

  17. ilevkivskyi commented on Feb 27, 2019

    @ilevkivskyi
    Member

    Related #1506 and #1507 (I am not sure we need to keep all three open)

  18. ssbarnea commented on Dec 14, 2020

    @ssbarnea
    SponsorContributor

    I think it worth keeping this open until mypy documentation will contain an example on how to use typing with a simple @contextmanager, like one that takes care of chdir maybe?

  19. JukkaL commented on Apr 20, 2022

    @JukkaL
    Collaborator

    Lowering priority since arguably the most common case is now supported by using ParamSpec:

    from typing import Callable, TypeVar, Iterator
    from typing_extensions import ParamSpec
    
    T = TypeVar("T")
    P = ParamSpec("P")
    
    def transform(f: Callable[P, T]) -> Callable[P, Iterator[T]]:
        ...
    
    def func(x: T) -> T:
        ...
    
    reveal_type(transform(func))  # Revealed type is 'def [T] (x: T`-1) -> typing.Iterator[T`-1]'
  20. added a commit that references this issue on Jun 18, 2023
    0873230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions