Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
asottile opened this issue Jan 30, 2019 · 10 comments
Labels
status: deferred Issue or PR deferred until some precondition is fixed stubs: false negative Type checkers do not report an error, but should

Comments

@asottile
Copy link
Contributor

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 srittau added size-small stubs: false negative Type checkers do not report an error, but should labels Jan 30, 2019
@srittau
Copy link
Collaborator

srittau commented Jan 30, 2019

Sounds reasonable.

@graingert
Copy link
Contributor

graingert commented Jun 30, 2021

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
Copy link
Contributor Author

asottile commented Mar 1, 2022

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

@HacKanCuBa
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Member

Marking as deferred unless/until PEP 696 is accepted, as per #7430 (comment).

@srittau
Copy link
Collaborator

srittau commented Feb 14, 2024

See #11422 for the type var generics feature tracker.

@hauntsaninja
Copy link
Collaborator

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 #7430 (comment) 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: #7430 (comment)

@Akuli Akuli added status: deferred Issue or PR deferred until some precondition is fixed and removed status: deferred Issue or PR deferred until some precondition is fixed labels Jun 3, 2024
@max-muoto
Copy link
Contributor

Now that we have PEP 696, a PR experimenting with the effects here: #12334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: deferred Issue or PR deferred until some precondition is fixed stubs: false negative Type checkers do not report an error, but should
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants