python / typeshed

Collection of library stubs for Python, with static types
Other
4.38k stars 1.76k forks source link

contextlib.contextmanager allows `Iterator[T]` -- should probably be `Generator[T, None, None]`? #2772

Open asottile opened 5 years ago

asottile commented 5 years ago

For instance, this script passes mypy:

import contextlib
from typing import Iterator

@contextlib.contextmanager
def f() -> Iterator[int]:
    return iter([1])

with f():
    pass

with f():
    raise TypeError('wat')

but fails at runtime (and not in the expected way):

$ python t.py 
Traceback (most recent call last):
  File "t.py", line 14, in <module>
    raise TypeError('wat')
TypeError: wat

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "t.py", line 14, in <module>
    raise TypeError('wat')
  File "/usr/lib/python3.6/contextlib.py", line 99, in __exit__
    self.gen.throw(type, value, traceback)
AttributeError: 'list_iterator' object has no attribute 'throw'
srittau commented 5 years ago

Sounds reasonable.

graingert commented 3 years ago

contextlib.contextmanager doesn't need to be as specific as Generator[T, None, None], it could take the slightly wider type:

class ContextManagerArgReturnType(Protocol, Generic[T]):
    def __next__(self) -> T: ...
    def throw(self, exctype: Optional[Type[BaseException]], excinst: Optional[BaseException], exctb: Optional[TracebackType]) -> Any: ...

ContextManagerArgType = Callable[[], ContextManagerArgReturnType[T]]
asottile commented 2 years ago

this (unfortunately) caused a production incident today -- any chance this can be reconsidered (despite the backward incompatibility)?

HacKanCuBa commented 2 years ago

So, this is an interesting pitfall of using Iterator[] instead of Generator[] as the docs suggest:

A generator can be annotated by the generic type Generator[YieldType, SendType, ReturnType] ... Alternatively, annotate your generator as having a return type of either Iterable[YieldType] or Iterator[YieldType]

I'm just bumping this up.

AlexWaygood commented 2 years ago

So, this is an interesting pitfall of using Iterator[] instead of Generator[] as the docs suggest:

A generator can be annotated by the generic type Generator[YieldType, SendType, ReturnType] ... Alternatively, annotate your generator as having a return type of either Iterable[YieldType] or Iterator[YieldType]

I'm just bumping this up.

Can I ask why you're bumping it up? Did the current stubs mean that mypy failed to spot a bug in your code, the same as happened to @asottile?

HacKanCuBa commented 2 years ago

So, this is an interesting pitfall of using Iterator[] instead of Generator[] as the docs suggest:

A generator can be annotated by the generic type Generator[YieldType, SendType, ReturnType] ... Alternatively, annotate your generator as having a return type of either Iterable[YieldType] or Iterator[YieldType]

I'm just bumping this up.

Can I ask why you're bumping it up? Did the current stubs mean that mypy failed to spot a bug in your code, the same as happened to @asottile?

Which brought me here after reading a blogpost :D

AlexWaygood commented 1 year ago

Marking as deferred unless/until PEP 696 is accepted, as per https://github.com/python/typeshed/pull/7430#issuecomment-1238711970.

srittau commented 9 months ago

See #11422 for the type var generics feature tracker.

hauntsaninja commented 9 months ago

Note that while PEP 696 will help some ergonomic issues and therefore could be good to re-assess, I'm still pretty strongly disinclined to change this. It's been 1.5 years since I commented in https://github.com/python/typeshed/pull/7430#issuecomment-1238711970 asking people to report real-life false negatives and no one else has (I just went through all the issues that link to this one and I don't remember any mypy issue reports either). A ruff lint with autofix could help, as would Jukka's suggestion here: https://github.com/python/typeshed/pull/7430#issuecomment-1080460175

max-muoto commented 4 months ago

Now that we have PEP 696, a PR experimenting with the effects here: https://github.com/python/typeshed/pull/12334