GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — require Generator for `contextmanager` by asottile · Pull Request #7430 · python/typeshed

Conversation

asottile commented Mar 3, 2022

Copy link

Copy Markdown

Contributor

re-try of #2772 / #2773

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

graingert and jonathanslenders reacted with heart emoji

All reactions

❤️ 2 reactions

This comment has been minimized.

asottile commented Mar 3, 2022

Copy link

Copy Markdown

Contributor Author

just to gauge impact here, while there's a lot of repositories with this mistake, the fix is pretty easy. here's black for instance:

$ git diff | cat
diff --git a/src/black/__init__.py b/src/black/__init__.py
index c4ec99b..e6f4e80 100644
--- a/src/black/__init__.py
+++ b/src/black/__init__.py
@@ -19,7 +19,6 @@ from typing import (
     Any,
     Dict,
     Generator,
-    Iterator,
     List,
     MutableMapping,
     Optional,
@@ -1405,7 +1404,7 @@ def assert_stable(src: str, dst: str, mode: Mode) -> None:
 
 
 @contextmanager
-def nullcontext() -> Iterator[None]:
+def nullcontext() -> Generator[None, None, None]:
     """Return an empty context manager.
 
     To be used like `nullcontext` in Python 3.7.
diff --git a/src/blib2to3/pgen2/driver.py b/src/blib2to3/pgen2/driver.py
index 8fe8206..5f8a905 100644
--- a/src/blib2to3/pgen2/driver.py
+++ b/src/blib2to3/pgen2/driver.py
@@ -24,12 +24,12 @@ import sys
 from typing import (
     Any,
     cast,
+    Generator,
     IO,
     Iterable,
     List,
     Optional,
     Text,
-    Iterator,
     Tuple,
     TypeVar,
     Generic,
@@ -66,7 +66,7 @@ class TokenProxy:
         self._release_ranges: List[ReleaseRange] = []
 
     @contextmanager
-    def release(self) -> Iterator["TokenProxy"]:
+    def release(self) -> Generator["TokenProxy", None, None]:
         release_range = ReleaseRange(self._counter)
         self._release_ranges.append(release_range)
         try:
diff --git a/src/blib2to3/pgen2/parse.py b/src/blib2to3/pgen2/parse.py
index a9dc11f..f7c31bf 100644
--- a/src/blib2to3/pgen2/parse.py
+++ b/src/blib2to3/pgen2/parse.py
@@ -17,13 +17,13 @@ from . import grammar, token, tokenize
 from typing import (
     cast,
     Any,
+    Generator,
     Optional,
     Text,
     Union,
     Tuple,
     Dict,
     List,
-    Iterator,
     Callable,
     Set,
     TYPE_CHECKING,
@@ -72,7 +72,7 @@ class Recorder:
         return self._dead_ilabels.symmetric_difference(self._ilabels)
 
     @contextmanager
-    def switch_to(self, ilabel: int) -> Iterator[None]:
+    def switch_to(self, ilabel: int) -> Generator[None, None, None]:
         with self.backtrack():
             self.parser.stack = self._points[ilabel]
             try:
@@ -83,7 +83,7 @@ class Recorder:
                 self.parser.stack = self._start_point
 
     @contextmanager
-    def backtrack(self) -> Iterator[None]:
+    def backtrack(self) -> Generator[None, None, None]:
         """
         Use the node-level invariant ones for basic parsing operations (push/pop/shift).
         These still will operate on the stack; but they won't create any new nodes, or

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood commented Mar 3, 2022

Copy link

Copy Markdown

Member

Arguably the mypy docs would also have to be updated:

A basic generator that only yields values can be succinctly annotated as having a return type of either Iterator[YieldType] or Iterable[YieldType].

There's also similar guidance in the CPython docs for typing.Generator.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

asottile commented Mar 3, 2022

Copy link

Copy Markdown

Contributor Author

Arguably the mypy docs would also have to be updated:

A basic generator that only yields values can be succinctly annotated as having a return type of either Iterator[YieldType] or Iterable[YieldType].

There's also similar guidance in the CPython docs for typing.Generator.

I don't think either of those need updates -- this is specifically for context managers (the problem here is a Generator is an Iterator, but an Iterator is not a Generator)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

srittau commented Mar 3, 2022

Copy link

Copy Markdown

Collaborator

I know this is quite controversial, but I would like to see this go in rather sooner than later. The longer we wait the worse it gets, and our stubs are arguably wrong.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra commented Mar 3, 2022

Copy link

Copy Markdown

Member

I also agree we should do this, but it's definitely going to be quite disruptive. mypy-primer is only going to show a fraction of the real-world code that needs to be changed.

I agree with @AlexWaygood that we should call this out explicitly in the mypy and typing docs; @contextmanager is one of the most common uses of generators and people will get confused by this error. I can help get these docs updates merged.

@asottile do you think it's possible to provide a codemod that fixes these annotations?

AlexWaygood and graingert reacted with thumbs up emoji

All reactions

👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood commented Mar 3, 2022

Copy link

Copy Markdown

Member

^I started typing out a response, but (as is so often the case) @JelleZijlstra beat me to it, and I agree with everything he said :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

asottile commented Mar 3, 2022

Copy link

Copy Markdown

Contributor Author

I also agree we should do this, but it's definitely going to be quite disruptive. mypy-primer is only going to show a fraction of the real-world code that needs to be changed.

I agree with @AlexWaygood that we should call this out explicitly in the mypy and typing docs; @contextmanager is one of the most common uses of generators and people will get confused by this error. I can help get these docs updates merged.

@asottile do you think it's possible to provide a codemod that fixes these annotations?

here's the start of one, based on the approach outlined here (same as pyupgrade / add-trailing-comma / yesqa / etc. work):

from __future__ import annotations

import argparse
import ast
import sys
from typing import Sequence

import tokenize_rt


def is_ctxmanager(node: ast.AST, name: str) -> bool:
    return (
        isinstance(node, ast.Name) and node.id == name
    ) or (
        isinstance(node, ast.Attribute) and
        node.attr == name and
        # simple name, might be an aliased import :shrugs:
        isinstance(node.value, ast.Name)
    )


def is_iterator(node: ast.AST, name: str) -> bool:
    return (
        isinstance(node, ast.Subscript) and (
            (
                isinstance(node.value, ast.Name) and
                node.value.id == name
            ) or (
                isinstance(node.value, ast.Attribute) and
                node.value.attr == name and
                # simple name, might be an aliased import :shrugs:
                isinstance(node.value.value, ast.Name)
            )
        )
    )


class Visitor(ast.NodeVisitor):
    def __init__(self) -> None:
        self.iter_to_generator: set[tokenize_rt.Offset] = set()
        self.aiter_to_agenerator: set[tokenize_rt.Offset] = set()

    def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
        if (
                len(node.decorator_list) == 1 and
                is_ctxmanager(node.decorator_list[0], 'contextmanager') and
                node.returns is not None and
                is_iterator(node.returns, 'Iterator')
        ):
            offset = tokenize_rt.Offset(
                node.returns.lineno,
                node.returns.col_offset,
            )
            self.iter_to_generator.add(offset)
        self.generic_visit(node)

    def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None:
        if (
                len(node.decorator_list) == 1 and
                is_ctxmanager(node.decorator_list[0], 'asynccontextmanager') and
                node.returns is not None and
                is_iterator(node.returns, 'AsyncIterator')
        ):
            offset = tokenize_rt.Offset(
                node.returns.lineno,
                node.returns.col_offset,
            )
            self.aiter_to_agenerator.add(offset)
        self.generic_visit(node)


def _fix(
        i: int,
        tokens: list[tokenize_rt.Token],
        n: int,
        orig: str,
        dest: str,
) -> None:
    j = i
    try:
        while tokens[i].src != orig:
            i += 1
    except Exception:
        breakpoint()
        raise

    tokens[i] = tokens[i]._replace(src=dest)

    while tokens[i].src != '[':
        i += 1

    i += 1
    depth = 1
    while depth:
        if tokens[i].src in '([{':
            depth += 1
        elif tokens[i].src in ')]}':
            depth -= 1

        i += 1
    tokens[i - 1] = tokens[i - 1]._replace(src=f'{", None" * n}]')


def _fix_file(filename: str) -> int:
    with open(filename, 'rb') as fb:
        contents_bytes = fb.read()

    try:
        contents_text_orig = contents_text = contents_bytes.decode()
    except UnicodeDecodeError:
        print(f'{filename} is non-utf-8 (not supported)')
        return 1

    tree = ast.parse(contents_text, filename=filename)
    visitor = Visitor()
    visitor.visit(tree)

    if not visitor.iter_to_generator and not visitor.aiter_to_agenerator:
        return 0

    tokens = tokenize_rt.src_to_tokens(contents_text)
    for i, token in tokenize_rt.reversed_enumerate(tokens):
        if token.offset in visitor.iter_to_generator:
            _fix(i, tokens, n=2, orig='Iterator', dest='Generator')
        elif token.offset in visitor.aiter_to_agenerator:
            _fix(i, tokens, n=1, orig='AsyncIterator', dest='AsyncGenerator')

    contents_text = tokenize_rt.tokens_to_src(tokens)
    if contents_text == contents_text_orig:
        return 0

    print(f'Rewriting {filename}')
    with open(filename, 'w', encoding='UTF-8', newline='') as f:
        f.write(contents_text)
    return 1


def main(argv: Sequence[str] | None = None) -> int:
    parser = argparse.ArgumentParser()
    parser.add_argument('filenames', nargs='*')
    args = parser.parse_args(argv)

    if sys.version_info < (3, 10):
        raise SystemExit(
            'this script makes some assumptions about ast.Subscript and '
            'needs python 3.10+',
        )

    ret = 0
    for filename in args.filenames:
        ret |= _fix_file(filename)
    return ret


if __name__ == '__main__':
    raise SystemExit(main())

it doesn't fix the imports, but that's tricky to do accurately and with respecting the user's style

import contextlib
from contextlib import contextmanager
from contextlib import asynccontextmanager
from typing import AsyncIterator, Iterator
import typing
import typing as t


def fn_unrelated() -> Iterator[None]:
    yield

@contextlib.contextmanager
def ctx1() -> Iterator[None]:
    yield

@contextmanager
def ctx2() -> Iterator[None]:
    yield

@contextlib.asynccontextmanager
async def ctx3() -> AsyncIterator[None]:
    yield

@asynccontextmanager
async def ctx4() -> AsyncIterator[None]:
    yield

@contextlib.contextmanager
def ctx5() -> typing.Iterator[None]:
    yield

@contextlib.contextmanager
def ctx6() -> t.Iterator[None]:
    yield

$ diff -u t2.py.bak t2.py
--- t2.py.bak	2022-03-03 14:24:22.591697064 -0500
+++ t2.py	2022-03-03 14:42:18.439722185 -0500
@@ -10,25 +10,25 @@
     yield
 
 @contextlib.contextmanager
-def ctx1() -> Iterator[None]:
+def ctx1() -> Generator[None, None, None]:
     yield
 
 @contextmanager
-def ctx2() -> Iterator[None]:
+def ctx2() -> Generator[None, None, None]:
     yield
 
 @contextlib.asynccontextmanager
-async def ctx3() -> AsyncIterator[None]:
+async def ctx3() -> AsyncGenerator[None, None]:
     yield
 
 @asynccontextmanager
-async def ctx4() -> AsyncIterator[None]:
+async def ctx4() -> AsyncGenerator[None, None]:
     yield
 
 @contextlib.contextmanager
-def ctx5() -> typing.Iterator[None]:
+def ctx5() -> typing.Generator[None, None, None]:
     yield
 
 @contextlib.contextmanager
-def ctx6() -> t.Iterator[None]:
+def ctx6() -> t.Generator[None, None, None]:
     yield

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hauntsaninja commented Mar 3, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

Seems like I'm in the minority, but I'm -1 on this.

Breaking almost every use case of contextmanager for something pretty niche is too much for me.

Other than your reports, I haven't encountered instances of people running into this. There are zero emojis on the issue report. There are two out links, neither of which seems to actually have the bug in question. Maybe a failure of my imagination, but I'm also struggling to think of the real world code that motivates someone to return an iterator here.

On the other hand, it's very common for users with correct code to get confused when adding types for Iterable, Iterator, Generator. By steering users to use the scary and not ergonomic Generator, this is a real loss of usability in the 99.99% use case going forward as well. There's a reason there's so much code out there that this would break.

Akuli reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood commented Mar 3, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

It's a shame we don't have something like the following construct in typing.py:

_T_co = TypeVar("_T_co", covariant=True)

SimpleGenerator = Generator[_T_co, None, None]

Returning SimpleGenerator[_T_co] from these functions would be much more correct than returning Iterator[_T_co], and much more user-friendly than having to type out Generator[_T_co, None, None] every time.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra commented Mar 3, 2022

Copy link

Copy Markdown

Member

it would be a bit nicer if we had TypeVar defaults so we could just write Generator[T].

hauntsaninja, jonathanslenders, and cj81499 reacted with thumbs up emoji

All reactions

👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hauntsaninja commented Mar 3, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

Also note that a linter could warn for contextmanager decorated functions that don't yield. Could be a good check to add to pylint or flake8-bugbear. Since this wouldn't look at annotations, it'd side step the issue of forcing users to change their annotations.

AlexWaygood reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

asottile commented Mar 3, 2022

Copy link

Copy Markdown

Contributor Author

the case I had would not have been detected by that -- it returned an iterable from another function call. I think this really needs type checkers to do the proper thing

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood commented Mar 4, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

the case I had would not have been detected by that -- it returned an iterable from another function call. I think this really needs type checkers to do the proper thing

@asottile I'm not sure I understand. If you have a function like this:

@contextmanager
def func():
    return other_generator_function()

Then a flake8 plugin could easily require that you do this instead, ensuring type safety:

@contextmanager
def func():
    yield from other_generator_function()

It doesn't need to do any fancy guesswork about whether other_generator_function is actually a generator function, it just needs to make sure that func yields somewhere, no?

We can agree that this would definitely be an imperfect solution, but it would be far less disruptive.

hauntsaninja reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Akuli commented Mar 4, 2022

Copy link

Copy Markdown

Collaborator

Generator[T] isn't great, because typing.py in standard library of released Pythons doesn't support it at runtime, and most people don't use from __future__ import annotations. This means we're stuck with Iterator[T] or Generator[T, None, None].

I prefer Iterator[T] because it's simple. You should not have to know about sending anything into a generator just to make a context manager, and having to know about iterators is already confusing enough. To me, Iterator[T] satisfies "practicality beats purity", "simple is better than complex", and also "prefer false negatives".

Another possible solution would be mypy automatically inferring the correct return type for functions that yield, but I don't think that's ever going to happen because mypy has a concept of typed and untyped functions, and def foo(): is untyped, regardless of whether or not it has decorators.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

srittau commented Mar 4, 2022

Copy link

Copy Markdown

Collaborator

I prefer Iterator[T] because it's simple.

But it's also confusing. A generator function return a generator after all (which happens to be an iterator as well).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Akuli commented Mar 4, 2022

Copy link

Copy Markdown

Collaborator

You really shouldn't have to care. The type annotation in this case should IMO be something simple that doesn't get in the way, not something full of None, None that makes you stop and think about it carefully.

AlexWaygood reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

srittau commented Mar 4, 2022

Copy link

Copy Markdown

Collaborator

I'm still in favor of allowing Generator[X] for cases like this. Iterator[X] is just the wrong annotation and as this discussion shows problematic in some cases.

henryiii, 93578237, graingert, laundmo, and jonathanslenders reacted with thumbs up emoji

All reactions

👍 5 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

graingert added a commit to agronholm/anyio that referenced this pull request

Mar 15, 2022


          contextlib.contextmanager takes a generator function

f570716

python/typeshed#7430

graingert mentioned this pull request

Mar 15, 2022

contextlib.contextmanager takes a generator function agronholm/anyio#428

Merged

agronholm pushed a commit to agronholm/anyio that referenced this pull request

Mar 15, 2022


          contextlib.contextmanager takes a generator function (#428)

eac5293

python/typeshed#7430

hauntsaninja commented Mar 21, 2022

Copy link

Copy Markdown

Collaborator

@asottile could you share more thoughts on why your case couldn't be caught by a simple and much less disruptive new linter check? It's unclear to at least Alex and I, see #7430 (comment)

AlexWaygood and Akuli reacted with thumbs up emoji

All reactions

👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

asottile commented Mar 21, 2022

Copy link

Copy Markdown

Contributor Author

@hauntsaninja I addressed that here: #7430 (comment)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

asottile commented Mar 21, 2022

Copy link

Copy Markdown

Contributor Author

additionally, the recommendation also does not work for async where there's no yield from or when there's branching logic (both of which are needed for the use case at work which caused an outage)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This comment has been minimized.

This comment was marked as outdated.

hauntsaninja pushed a commit to hauntsaninja/typeshed that referenced this pull request

Mar 21, 2022


          mypy_primer: mention if output is truncated

f5be781

When changes are really disruptive, like python#7430, we truncate output.
Mention this, so that it's known that the change is even more disruptive
than it might appear.

hauntsaninja mentioned this pull request

Mar 21, 2022

mypy_primer: mention if output is truncated #7525

Merged

asottile commented Mar 25, 2022

Copy link

Copy Markdown

Contributor Author

If 1000 projects that use type checking each need to spend 15 minutes to fix their codebase, that's 250 hours lost.

fwiw, one outage easily costs more than that

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JukkaL commented Mar 28, 2022

Copy link

Copy Markdown

Contributor

fwiw, one outage easily costs more than that

Yeah, it can cost more than than, but not every outage will be this expensive? It seems hard to estimate the likelihood of a change like this preventing an outage, though -- or is there an actual outage you're familiar with? The likelihood of many users having to update code is very high, since contextlib.contextmanager is very popular, and Generator doesn't seem to be often used as the return type of the decorated function.

I have an alternative proposal that would mostly solve the original issue and wouldn't require a lot of work from end users: special case the contextmanager decorator in type checkers, and complain only if it's decorating a non-generator function that doesn't return Generator[...]. This should still catch most real errors. Implementing this check in mypy would take maybe 3 hours of effort, and even if 3 type checkers would implement this, it could still be only ~10 hours of effort.

Type checkers would allow these:

@contextmanager
def f() -> Iterator[None]:
    yield None

@contextmanager
def f() -> Generator[None, None, None]:
    return <some generator>

But this wouldn't pass, since the decorated function isn't a generator function:

@contextmanager
def f() -> Iterator[None]:
    return iter(...)

AlexWaygood and uSpike reacted with thumbs up emoji

All reactions

👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

asottile commented Mar 28, 2022

Copy link

Copy Markdown

Contributor Author

fwiw, one outage easily costs more than that

Yeah, it can cost more than than, but not every outage will be this expensive?

this particular typing wart has already cost 3x more than your estimates above just from my experiences, and I'm sure I'm not alone in this. I also believe you're over-estimating the fix cost here, it is a simple find and replace to resolve the issue for the easiest case (and points out real bugs in the not-easiest case!). the typeshed stubs-no-longer-included migration easily dwarfs this in toil to mypy users. this is an exceptionally easy mistake to make even above while arguing against the change!.

and again, your suggestion won't help anything except for the most basic situation (which could already be detected by a linter)

my situation looks roughly like:

async def other_funtion_which_is_async_generator() -> AsyncIterator[...]:
    if some_precondition:
        return [...]  # bug was here
    elif ...:
        yield ...
        return ...

    yield ...
    return ...

@asynccontextmanger
async def ctx() -> AsyncIterator[...]:
   if some_complicated_condition:
        return other_function_which_is_async_generator()
   elif other_condition:
        async with ..."
            yield ...
            return

   yield ...
   return

unless your suggestion is to transparently change all "returns iterator but is a generator" functions from:

def f() -> Iterator[T]:
    yield ...

def f() -> Generator[T, None, None]:
    yield ...

then I don't think it will help (and even then I'm not sure it helps my case given it's not yielding on all branches but returning another function)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JukkaL commented Mar 28, 2022

Copy link

Copy Markdown

Contributor

@asottile Thanks for the additional context! This change now makes more sense to me.

I also believe you're over-estimating the fix cost here, it is a simple find and replace to resolve the issue for the easiest case

Generally imports need to be adjusted as well to update code, which is somewhat hard to to do using find/replace tools.

I agree that the fix is quite easy, but figuring out what needs to be done, and investigating whether this is a false positive or a mypy issue + the required context switch, creating a PR/change request, possibly getting it reviewed, waiting for CI, etc. is included in my 15 minute estimate (which is still pretty arbitrary).

To reduce the toil, we could add a dedicated error message about the non-Generator return type with clear instructions about what needs to be done. Providing a tool to automatically upgrade simple cases in affected code would further help, if it could also update imports.

the typeshed stubs-no-longer-included migration easily dwarfs this in toil to mypy users.

Yes, we knew that to have a big cost. We decided to move forward with it since not doing it risked making every future mypy upgrade painful for larger projects (mainly due to requiring all third-party stubs to be upgraded in lockstep with mypy). We try to avoid similar big breakages whenever there is a good alternative. That's why I'm trying to figure out if there is something less disruptive we could do here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

93578237 mentioned this pull request

Apr 4, 2022

Replace Iterator[T] with Generator[T,None, None] pypa/pip#11007

Merged

This comment was marked as outdated.

jonathanslenders commented Jun 16, 2022

Copy link

Copy Markdown

Contributor

Thanks for considering this change.

I came here after learning that I have to wrap async generators in an contextlib.aclosing block to ensure that finally blocks get properly executed, even if it contains an await. See: https://trio.readthedocs.io/en/stable/reference-core.html#finalization

Unfortunately, aclosing() doesn't work for AsyncIterable[T] but it does for AsyncGenerator[T, None], so I ended up replacing all occurances, and at the same time wondered about contextmanagers.

aclosing is here somewhat similar to asynccontextmanager. Being more precise in the return type seems to be always better, unfortunately it's more typing in this case.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This comment was marked as outdated.

AlexWaygood added the status: needs decision Needs a final decision by the typeshed maintainers label

Aug 20, 2022

twoertwein mentioned this pull request

Sep 4, 2022

TYP: contextmanager expects a Generator pandas-dev/pandas#48383

Merged


          require Generator for contextmanager

640ad29

github-actions Bot commented Sep 4, 2022

Copy link

Copy Markdown

Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

vision (https://github.com/pytorch/vision)
+ torchvision/datasets/imagenet.py:127: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[str]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ torchvision/datasets/imagenet.py:142: error: Need type annotation for "tmp_dir"  [var-annotated]

pegen (https://github.com/we-like-parsers/pegen)
+ src/pegen/parser_generator.py:64: error: Argument 1 to "contextmanager" has incompatible type "Callable[[ParserGenerator], Iterator[None]]"; expected "Callable[[ParserGenerator], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/pegen/parser_generator.py:78: error: Argument 1 to "contextmanager" has incompatible type "Callable[[ParserGenerator], Iterator[None]]"; expected "Callable[[ParserGenerator], Generator[<nothing>, Any, Any]]"  [arg-type]

porcupine (https://github.com/Akuli/porcupine)
+ porcupine/textutils.py:445: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Text], Iterator[None]]"; expected "Callable[[Text], Generator[<nothing>, Any, Any]]"  [arg-type]

streamlit (https://github.com/streamlit/streamlit)
+ lib/streamlit/runtime/caching/cache_utils.py: note: In class "CacheWarningCallStack":
+ lib/streamlit/runtime/caching/cache_utils.py:285:6: error: Argument 1 to "contextmanager" has incompatible type "Callable[[CacheWarningCallStack, FunctionType], Iterator[None]]"; expected "Callable[[CacheWarningCallStack, FunctionType], Generator[<nothing>, Any, Any]]"  [arg-type]
+ lib/streamlit/runtime/caching/cache_utils.py:293:6: error: Argument 1 to "contextmanager" has incompatible type "Callable[[CacheWarningCallStack], Iterator[None]]"; expected "Callable[[CacheWarningCallStack], Generator[<nothing>, Any, Any]]"  [arg-type]
+ lib/streamlit/runtime/caching/cache_utils.py: note: In class "CacheMessagesCallStack":
+ lib/streamlit/runtime/caching/cache_utils.py:389:6: error: Argument 1 to "contextmanager" has incompatible type "Callable[[CacheMessagesCallStack], Iterator[None]]"; expected "Callable[[CacheMessagesCallStack], Generator[<nothing>, Any, Any]]"  [arg-type]
+ lib/streamlit/runtime/legacy_caching/caching.py: note: At top level:
+ lib/streamlit/runtime/legacy_caching/caching.py:165:2: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Callable[..., Any]], Iterator[None]]"; expected "Callable[[Callable[..., Any]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ lib/streamlit/runtime/legacy_caching/caching.py:174:2: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ lib/streamlit/runtime/caching/__init__.py:72:2: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ lib/streamlit/__init__.py:393:2: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str], Iterator[None]]"; expected "Callable[[str], Generator[<nothing>, Any, Any]]"  [arg-type]

jax (https://github.com/google/jax)
+ jax/_src/config.py:902: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ jax/_src/config.py:913: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ jax/_src/config.py:1006: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str], Iterator[None]]"; expected "Callable[[str], Generator[<nothing>, Any, Any]]"  [arg-type]
+ jax/_src/source_info_util.py:198: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str], Iterator[NameStack]]"; expected "Callable[[str], Generator[<nothing>, Any, Any]]"  [arg-type]
+ jax/_src/source_info_util.py:209: error: Argument 1 to "contextmanager" has incompatible type "Callable[[NameStack], Iterator[None]]"; expected "Callable[[NameStack], Generator[<nothing>, Any, Any]]"  [arg-type]
+ jax/_src/source_info_util.py:219: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ jax/_src/source_info_util.py:224: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str], Iterator[NameStack]]"; expected "Callable[[str], Generator[<nothing>, Any, Any]]"  [arg-type]

mypy-protobuf (https://github.com/dropbox/mypy-protobuf)
+ mypy_protobuf/main.py:229: error: Argument 1 to "contextmanager" has incompatible type "Callable[[PkgWriter], Iterator[None]]"; expected "Callable[[PkgWriter], Generator[<nothing>, Any, Any]]"  [arg-type]
+ mypy_protobuf/main.py:973: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[Tuple[CodeGeneratorRequest, CodeGeneratorResponse]]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ mypy_protobuf/main.py:1005: error: <nothing> object is not iterable  [misc]
+ mypy_protobuf/main.py:1007: error: Cannot determine type of "request"  [has-type]
+ mypy_protobuf/main.py:1008: error: Cannot determine type of "response"  [has-type]
+ mypy_protobuf/main.py:1009: error: Cannot determine type of "request"  [has-type]
+ mypy_protobuf/main.py:1010: error: Cannot determine type of "request"  [has-type]
+ mypy_protobuf/main.py:1011: error: Cannot determine type of "request"  [has-type]
+ mypy_protobuf/main.py:1017: error: <nothing> object is not iterable  [misc]
+ mypy_protobuf/main.py:1019: error: Cannot determine type of "request"  [has-type]
+ mypy_protobuf/main.py:1020: error: Cannot determine type of "response"  [has-type]
+ mypy_protobuf/main.py:1021: error: Cannot determine type of "request"  [has-type]
+ mypy_protobuf/main.py:1022: error: Cannot determine type of "request"  [has-type]
+ mypy_protobuf/main.py:1023: error: Cannot determine type of "request"  [has-type]

black (https://github.com/psf/black)
+ src/blib2to3/pgen2/parse.py:74:6: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Recorder, int], Iterator[None]]"; expected "Callable[[Recorder, int], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/blib2to3/pgen2/parse.py:85:6: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Recorder], Iterator[None]]"; expected "Callable[[Recorder], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/blib2to3/pgen2/driver.py:68:6: error: Argument 1 to "contextmanager" has incompatible type "Callable[[TokenProxy], Iterator[TokenProxy]]"; expected "Callable[[TokenProxy], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/black/__init__.py:1334:2: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]

kopf (https://github.com/nolar/kopf)
+ kopf/_core/actions/invocation.py:73: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Iterable[Tuple[ContextVar[Any], Any]]], Iterator[None]]"; expected "Callable[[Iterable[Tuple[ContextVar[Any], Any]]], Generator[<nothing>, Any, Any]]"
+ kopf/_core/actions/execution.py:193: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[], AsyncIterator[None]]"; expected "Callable[[], AsyncGenerator[<nothing>, Any]]"
+ kopf/_cogs/clients/watching.py:71: error: Need type annotation for "operator_pause_waiter"
+ kopf/_cogs/clients/watching.py:89: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[NamedArg(Resource, 'resource'), NamedArg(Optional[NamespaceName], 'namespace'), DefaultNamedArg(Optional[ToggleSet], 'operator_paused')], AsyncIterator[Future[Any]]]"; expected "Callable[[NamedArg(Resource, 'resource'), NamedArg(Optional[NamespaceName], 'namespace'), DefaultNamedArg(Optional[ToggleSet], 'operator_paused')], AsyncGenerator[<nothing>, Any]]"
+ kopf/_core/reactor/subhandling.py:17: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[], AsyncIterator[None]]"; expected "Callable[[], AsyncGenerator[<nothing>, Any]]"

isort (https://github.com/pycqa/isort)
+ isort/io.py:55: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Union[str, Path]], Iterator[File]]"; expected "Callable[[Union[str, Path]], Generator[<nothing>, Any, Any]]"
+ isort/deprecated/finders.py:40: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str], Iterator[None]]"; expected "Callable[[str], Generator[<nothing>, Any, Any]]"
+ isort/api.py:337: error: Need type annotation for "source_file"
+ isort/api.py:353: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[TextIO]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"
+ isort/api.py:358: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Union[str, Path], File], Iterator[TextIO]]"; expected "Callable[[Union[str, Path], File], Generator[<nothing>, Any, Any]]"
+ isort/api.py:405: error: Need type annotation for "source_file"
+ isort/api.py:423: error: Need type annotation for "output_stream_context"
+ isort/api.py:591: error: Need type annotation for "source_file"
+ isort/pylama_isort.py:13: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"

pandas (https://github.com/pandas-dev/pandas)
+ pandas/util/_exceptions.py:10: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str, str], Iterator[None]]"; expected "Callable[[str, str], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_config/config.py:745: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Any], Iterator[None]]"; expected "Callable[[Any], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_config/localization.py:20: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Union[str, Tuple[str, str]], int], Iterator[Union[str, Tuple[str, str]]]]"; expected "Callable[[Union[str, Tuple[str, str]], int], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/core/common.py:536: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Any, str, Any], Iterator[None]]"; expected "Callable[[Any, str, Any], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/plotting/_misc.py:596: error: Argument 1 to "contextmanager" has incompatible type "Callable[[_Options, Any, Any], Iterator[_Options]]"; expected "Callable[[_Options, Any, Any], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/io/formats/format.py:1208: error: Need type annotation for "f"  [var-annotated]
+ pandas/io/formats/format.py:1212: error: Unused "type: ignore" comment
+ pandas/io/formats/format.py:1216: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Union[Union[str, PathLike[str]], WriteBuffer[str], None], Optional[str]], Union[Iterator[WriteBuffer[str]], Iterator[StringIO]]]"; expected "Callable[[Union[Union[str, PathLike[str]], WriteBuffer[str], None], Optional[str]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/core/groupby/groupby.py:1108: error: Argument 1 to "contextmanager" has incompatible type "Callable[[GroupBy[NDFrameT]], Iterator[GroupBy[Any]]]"; expected "Callable[[GroupBy[NDFrameT]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/compat/pickle_compat.py:296: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_testing/contexts.py:22: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Any, Any], Iterator[IO[bytes]]]"; expected "Callable[[Any, Any], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_testing/contexts.py:43: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str], Iterator[None]]"; expected "Callable[[str], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_testing/contexts.py:129: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[str]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_testing/contexts.py:148: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_testing/contexts.py:164: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Any, KwArg(Any)], Iterator[None]]"; expected "Callable[[Any, KwArg(Any)], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/_testing/contexts.py:198: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Any, Any], Iterator[None]]"; expected "Callable[[Any, Any], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/util/_test_decorators.py:259: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pandas/plotting/_matplotlib/converter.py:101: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]

pybind11 (https://github.com/pybind/pybind11)
+ setup.py:114: error: Argument 1 to "contextmanager" has incompatible type "Callable[[VarArg(str)], Iterator[None]]"; expected "Callable[[VarArg(str)], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pybind11/setup_helpers.py:215: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[str]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]

paasta (https://github.com/yelp/paasta)
+ paasta_tools/utils.py:1657: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[None]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"
+ paasta_tools/utils.py:1759: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Union[IOBase, IO[Any]]], Iterator[None]]"; expected "Callable[[Union[IOBase, IO[Any]]], Generator[<nothing>, Any, Any]]"
+ paasta_tools/utils.py:1768: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Union[IOBase, IO[Any]], int], Iterator[None]]"; expected "Callable[[Union[IOBase, IO[Any]], int], Generator[<nothing>, Any, Any]]"
+ paasta_tools/utils.py:1774: error: Need type annotation for "flock_context"
+ paasta_tools/utils.py:2828: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str], Iterator[IO[Any]]]"; expected "Callable[[str], Generator[<nothing>, Any, Any]]"
+ paasta_tools/bounce_lib.py:132: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str, Optional[SystemPaastaConfig]], Iterator[Any]]"; expected "Callable[[str, Optional[SystemPaastaConfig]], Generator[<nothing>, Any, Any]]"
+ paasta_tools/generate_deployments_for_service.py:236: error: Need type annotation for "newf"
+ paasta_tools/autoscaling/autoscaling_service_lib.py:984: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str, str], Iterator[None]]"; expected "Callable[[str, str], Generator[<nothing>, Any, Any]]"

core (https://github.com/home-assistant/core)
+ homeassistant/components/fjaraskupan/__init__.py:105: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[Coordinator], AsyncIterator[Any]]"; expected "Callable[[Coordinator], AsyncGenerator[<nothing>, Any]]"  [arg-type]
+ homeassistant/components/file_upload/__init__.py:31: error: Argument 1 to "contextmanager" has incompatible type "Callable[[HomeAssistant, str], Iterator[Path]]"; expected "Callable[[HomeAssistant, str], Generator[<nothing>, Any, Any]]"  [arg-type]
+ homeassistant/components/zha/config_flow.py:408: error: Need type annotation for "file_path"  [var-annotated]
+ homeassistant/components/script/trace.py:21: error: Argument 1 to "contextmanager" has incompatible type "Callable[[HomeAssistant, str, Dict[str, Any], Dict[str, Any], Context, Dict[str, Any]], Iterator[ScriptTrace]]"; expected "Callable[[HomeAssistant, str, Dict[str, Any], Dict[str, Any], Context, Dict[str, Any]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ homeassistant/components/fjaraskupan/number.py:61: error: Need type annotation for "device"  [var-annotated]
+ homeassistant/components/fjaraskupan/light.py:50: error: Need type annotation for "device"  [var-annotated]
+ homeassistant/components/fjaraskupan/light.py:60: error: Need type annotation for "device"  [var-annotated]
+ homeassistant/components/fjaraskupan/fan.py:88: error: Need type annotation for "device"  [var-annotated]
+ homeassistant/components/fjaraskupan/fan.py:109: error: Need type annotation for "device"  [var-annotated]
+ homeassistant/components/fjaraskupan/fan.py:126: error: Need type annotation for "device"  [var-annotated]
+ homeassistant/components/fjaraskupan/fan.py:133: error: Need type annotation for "device"  [var-annotated]
+ homeassistant/components/amcrest/__init__.py:208: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[AmcrestChecker, VarArg(Any), KwArg(Any)], AsyncIterator[Any]]"; expected "Callable[[AmcrestChecker, VarArg(Any), KwArg(Any)], AsyncGenerator[<nothing>, Any]]"  [arg-type]
+ homeassistant/components/amcrest/__init__.py:217: error: Argument 1 to "asynccontextmanager" has incompatible type "Callable[[AmcrestChecker], AsyncIterator[None]]"; expected "Callable[[AmcrestChecker], AsyncGenerator[<nothing>, Any]]"  [arg-type]

spark (https://github.com/apache/spark)
+ python/pyspark/pandas/utils.py:489: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Dict[str, Any], DefaultNamedArg(Optional[SparkSession], 'spark')], Iterator[None]]"; expected "Callable[[Dict[str, Any], DefaultNamedArg(Optional[SparkSession], 'spark')], Generator[<nothing>, Any, Any]]"  [arg-type]
+ python/pyspark/pandas/config.py:375: error: Argument 1 to "contextmanager" has incompatible type "Callable[[VarArg(Any)], Iterator[None]]"; expected "Callable[[VarArg(Any)], Generator[<nothing>, Any, Any]]"  [arg-type]

pyinstrument (https://github.com/joerick/pyinstrument)
- pyinstrument/vendor/decorator.py:295: error: Incompatible types in assignment (expression has type "Callable[[Any, Any, VarArg(Any), KwArg(Any)], Any]", variable has type "Callable[[_GeneratorContextManager[_T_co], Callable[..., Iterator[_T_co]], Tuple[Any, ...], Dict[str, Any]], None]")
+ pyinstrument/vendor/decorator.py:295: error: Incompatible types in assignment (expression has type "Callable[[Any, Any, VarArg(Any), KwArg(Any)], Any]", variable has type "Callable[[_GeneratorContextManager[_T_co], Callable[..., Generator[_T_co, Any, Any]], Tuple[Any, ...], Dict[str, Any]], None]")
- pyinstrument/vendor/decorator.py:301: error: Incompatible types in assignment (expression has type "Callable[[Any, Any, VarArg(Any), KwArg(Any)], Any]", variable has type "Callable[[_GeneratorContextManager[_T_co], Callable[..., Iterator[_T_co]], Tuple[Any, ...], Dict[str, Any]], None]")
+ pyinstrument/vendor/decorator.py:301: error: Incompatible types in assignment (expression has type "Callable[[Any, Any, VarArg(Any), KwArg(Any)], Any]", variable has type "Callable[[_GeneratorContextManager[_T_co], Callable[..., Generator[_T_co, Any, Any]], Tuple[Any, ...], Dict[str, Any]], None]")

poetry (https://github.com/python-poetry/poetry)
+ src/poetry/config/file_config_source.py:34: error: Need type annotation for "toml"  [var-annotated]
+ src/poetry/config/file_config_source.py:49: error: Need type annotation for "toml"  [var-annotated]
+ src/poetry/config/file_config_source.py:65: error: Argument 1 to "contextmanager" has incompatible type "Callable[[FileConfigSource], Iterator[Any]]"; expected "Callable[[FileConfigSource], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/utils/helpers.py:30: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Path], Iterator[Path]]"; expected "Callable[[Path], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/utils/helpers.py:89: error: Need type annotation for "update_context"  [var-annotated]
+ src/poetry/utils/env.py:1751: error: Argument 1 to "contextmanager" has incompatible type "Callable[[VirtualEnv], Iterator[None]]"; expected "Callable[[VirtualEnv], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/utils/env.py:1885: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Union[str, Path, None], Optional[Dict[str, bool]]], Iterator[VirtualEnv]]"; expected "Callable[[Union[str, Path, None], Optional[Dict[str, bool]]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/utils/env.py:1901: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Any, Optional[Env], Optional[Any]], Iterator[Env]]"; expected "Callable[[Any, Optional[Env], Optional[Any]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/utils/env.py:1913: error: Need type annotation for "venv"  [var-annotated]
+ src/poetry/inspection/info.py:578: error: Need type annotation for "venv"  [var-annotated]
+ src/poetry/puzzle/provider.py:66: error: Argument 1 to "contextmanager" has incompatible type "Callable[[], Iterator[Callable[[Optional[str]], None]]]"; expected "Callable[[], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/puzzle/provider.py:178: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Provider, Path], Iterator[Provider]]"; expected "Callable[[Provider, Path], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/puzzle/provider.py:188: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Provider, Env], Iterator[Provider]]"; expected "Callable[[Provider, Env], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/puzzle/provider.py:201: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Provider, Collection[Any]], Iterator[Provider]]"; expected "Callable[[Provider, Collection[Any]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/puzzle/solver.py:62: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Solver, Env], Iterator[None]]"; expected "Callable[[Solver, Env], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/puzzle/solver.py:104: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Solver], Iterator[None]]"; expected "Callable[[Solver], Generator[<nothing>, Any, Any]]"  [arg-type]
+ src/poetry/masonry/builders/editable.py:86: error: Need type annotation for "env"  [var-annotated]
+ src/poetry/console/commands/build.py:26: error: Need type annotation for "env"  [var-annotated]

pylint (https://github.com/pycqa/pylint)
+ pylint/lint/utils.py:88: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Sequence[str]], Iterator[None]]"; expected "Callable[[Sequence[str]], Generator[<nothing>, Any, Any]]"  [arg-type]
+ pylint/lint/pylinter.py:670: error: Need type annotation for "check_astroid_module"  [var-annotated]
+ pylint/lint/pylinter.py:719: error: Need type annotation for "check_astroid_module"  [var-annotated]
+ pylint/lint/pylinter.py:916: error: Argument 1 to "contextmanager" has incompatible type "Callable[[PyLinter], Iterator[Callable[[Any], Optional[bool]]]]"; expected "Callable[[PyLinter], Generator[<nothing>, Any, Any]]"  [arg-type]

ibis (https://github.com/ibis-project/ibis)
+ ibis/config.py:141: error: Argument 1 to "contextmanager" has incompatible type "Callable[[str, Any], Iterator[None]]"; expected "Callable[[str, Any], Generator[<nothing>, Any, Any]]"

jinja (https://github.com/pallets/jinja)
+ src/jinja2/compiler.py:1753: error: Argument 1 to "contextmanager" has incompatible type "Callable[[CodeGenerator, Union[Filter, Test], Frame, bool], Iterator[None]]"; expected "Callable[[CodeGenerator, Union[Filter, Test], Frame, bool], Generator[<nothing>, Any, Any]]"  [arg-type]

rotki (https://github.com/rotki/rotki)
+ rotkehlchen/db/upgrades/v34_v35.py:100: error: Need type annotation for "write_cursor"
+ rotkehlchen/db/upgrades/v33_v34.py:13: error: Need type annotation for "write_cursor"
+ rotkehlchen/globaldb/handler.py:1131: error: Need type annotation for "user_db_cursor"
+ rotkehlchen/assets/utils.py:100: error: Need type annotation for "cursor"
+ rotkehlchen/db/upgrades/v32_v33.py:485: error: Need type annotation for "cursor"
+ rotkehlchen/db/ledger_actions.py:138: error: Need type annotation for "cursor"
+ rotkehlchen/db/history_events.py:86: error: Need type annotation for "cursor"
+ rotkehlchen/db/history_events.py:120: error: Need type annotation for "cursor"
+ rotkehlchen/db/eth2.py:120: error: Need type annotation for "cursor"
+ rotkehlchen/db/eth2.py:227: error: Need type annotation for "cursor"
+ rotkehlchen/db/eth2.py:252: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/yearn/vaults.py:772: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/yearn/vaults.py:823: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/yearn/vaults.py:879: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/l2/loopring.py:540: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/balancer/balancer.py:319: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/balancer/balancer.py:1101: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/balancer/balancer.py:1121: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/adex/adex.py:516: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/adex/adex.py:756: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/adex/adex.py:896: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/adex/adex.py:972: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/aave/graph.py:808: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/interfaces/ammswap/ammswap.py:425: error: Need type annotation for "cursor"
+ rotkehlchen/chain/bitcoin/xpub.py:212: error: Need type annotation for "cursor"
+ rotkehlchen/chain/bitcoin/xpub.py:274: error: Need type annotation for "cursor"
+ rotkehlchen/balances/manual.py:87: error: Need type annotation for "cursor"
+ rotkehlchen/balances/manual.py:107: error: Need type annotation for "cursor"
+ rotkehlchen/balances/manual.py:124: error: Need type annotation for "cursor"
+ rotkehlchen/exchanges/exchange.py:288: error: Need type annotation for "cursor"
+ rotkehlchen/exchanges/exchange.py:339: error: Need type annotation for "cursor"
+ rotkehlchen/exchanges/exchange.py:401: error: Need type annotation for "cursor"
+ rotkehlchen/exchanges/exchange.py:458: error: Need type annotation for "cursor"
+ rotkehlchen/db/upgrade_manager.py:115: error: Need type annotation for "cursor"
+ rotkehlchen/db/upgrade_manager.py:170: error: Need type annotation for "cursor"
+ rotkehlchen/db/ethtx.py:224: error: Need type annotation for "cursor"
+ rotkehlchen/db/ethtx.py:466: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/nfts.py:201: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/nfts.py:271: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/nfts.py:287: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/sushiswap/sushiswap.py:223: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/eth2/eth2.py:91: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/eth2/eth2.py:148: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/eth2/eth2.py:266: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/eth2/eth2.py:477: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/aave/aave.py:204: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/aave/aave.py:311: error: Need type annotation for "cursor"
+ rotkehlchen/exchanges/kraken.py:1172: error: Need type annotation for "write_cursor"
+ rotkehlchen/chain/ethereum/modules/yearn/vaultsv2.py:347: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/yearn/vaultsv2.py:370: error: Need type annotation for "cursor"
+ rotkehlchen/chain/ethereum/modules/uniswap/uniswap.py:343: error: Need type annotation for "cursor"
+ rotkehlchen/exchanges/manager.py:120: error: Need type annotation for "cursor"
+ rotkehlchen/db/reports.py:87: error: Need type annotation for "cursor"
+ rotkehlchen/db/reports.py:131: error: Need type annotation for "cursor"
+ rotkehlchen/db/reports.py:242: error: Need type annotation for "cursor"
+ rotkehlchen/db/reports.py:267: error: Need type annotation for "cursor"
+ rotkehlchen/db/dbhandler.py:268: error: Need type annotation for "cursor"
+ rotkehlchen/db/dbhandler.py:536: error: Argument 1 to "contextmanager" has incompatible type "Callable[[DBHandler], Iterator[DBCursor]]"; expected "Callable[[DBHandler], Generator[<nothing>, Any, Any]]"
+ rotkehlchen/db/dbhandler.py:558: error: Argument 1 to "contextmanager" has incompatible type "Callable[[DBHandler], Iterator[DBCursor]]"; expected "Callable[[DBHandler], Generator[<nothing>, Any, Any]]"
+ rotkehlchen/db/dbhandler.py:607: error: Need type annotation for "cursor"
+ rotkehlchen/db/dbhandler.py:677: error: Need type annotation for "write_cursor"
+ rotkehlchen/db/dbhandler.py:1025: error: Need type annotation for "cursor"
+ rotkehlchen/db/dbhandler.py:1660: error: Need type annotation for "cursor"
+ rotkehlchen/db/dbhandler.py:2359: error: Need type annotation for "cursor"
+ rotkehlchen/db/dbhandler.py:2606: error: Need type annotation for "write_cursor"
+ rotkehlchen/db/dbhandler.py:3114: error: Need type annotation for "write_cursor"
+ rotkehlchen/db/dbhandler.py:3278: error: Need type annotation for "cursor"
+ rotkehlchen/db/dbhandler.py:3299: error: Need type

... (truncated 521 lines) ...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

srittau approved these changes

Sep 5, 2022

View reviewed changes

srittau left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Question for other maintainers: What is needed for this to go in?

Cc @JelleZijlstra @JukkaL

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

JelleZijlstra commented Sep 5, 2022

Copy link

Copy Markdown

Member

I think this is too disruptive until we have PEP-696 (TypeVar defaults).

hauntsaninja reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood commented Sep 5, 2022

Copy link

Copy Markdown

Member

I also think this is just too disruptive, I'm afraid :( I'd much prefer it if type checkers could add some special-casing for this, as @JukkaL suggested.

This is one of those occasions where it really should be solved in the stubs... but I just don't think the backwards-compatibility break is worth the cost (and I don't want to have to tell people they need to make their type hints much uglier every time they use @contextmanager, either).

Akuli reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

twoertwein commented Sep 6, 2022

Copy link

Copy Markdown

Contributor

The mypy-primer output looks disruptive but these cases are all easy to fix (for example in pandas pandas-dev/pandas#48383). Importantly, it is only necessary to fix the few(er) Argument 1 to "contextmanager" has incompatible type errors, as that should take care of all the Need type annotation for errors.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hauntsaninja added a commit to hauntsaninja/typeshed that referenced this pull request

Sep 6, 2022


          Require AsyncGenerator for asynccontextmanager

cd13dd0

I'm against merging python#7430 because:
- Generator is not an ergonomic type
- It's very widely breaking
- Safety could easily be enforced by a linter that ensures use of `yield
  from` instead of `return` in an @contextmanager

To expand on that a little:
- I care about the typing system being accessible. Generator with its
  three type vars is not a friendly type. I see enough confusion
  about Iterable, Iterator, Generator as it is.
- Maintaining a high signal to noise / effort ratio is an important
  part of making typing accessible. Breaking changes that seem
  arbitrary to a casual user reinforce existing negative impressions
  of typing that hurt the ecosystem as a whole.
- In all the years of typing, this has come up basically never
  (reported twice by asottile, none of the outlinks from the issue
  contain the bug, issue itself is not popular), so I think this is
  99.99% noise. Between typeshed and mypy, I've seen a lot of typing
  issue reports.

But I'm willing to admit that the considerations are slightly different
for asynccontextmanager:
- async is less widely used than sync, so maybe this is less
  widely breaking (let's at least see primer)
- async is typically used by more experienced Python users, so there's
  already an accessibility floor here
- There is no equivalent to `yield from` in async that a linter could
  enforce, so this genuinely can only be solved in type checkers
- The issue that led to python#7430 was with asynccontextmanager
- We've gotten away with some pedantic narrowings of async types before

This is just an experiment, I'm not sure whether I'm actually in favour
of this change, but I'm a lot more willing to consider it. Note that I'm
not open to slippery slope consistency arguments here; if a constraint
is that we do both async and sync or neither, then I'm firmly in the
neither camp.

hauntsaninja mentioned this pull request

Sep 6, 2022

Require AsyncGenerator for asynccontextmanager #8698

Closed

hauntsaninja commented Sep 6, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

There are currently three maintainers against and one maintainer for, so I'm closing this PR. We can think about revisiting if PEP 696 is accepted and implemented.

I do think the pros and cons are slightly different for contextmanager vs asynccontextmanager, so I'm willing to consider the asynccontextmanager change on its own. This is #8698

If you stumble upon this, I'm also interested in hearing about non-hypothetical cases where you actually have first-hand run into issues preventable by this; please comment on issue #2772 if so. Hearing about such cases helps us make informed decisions about the value of changes like this.

As is mentioned in this PR, if you're looking for safety in the sync case, a linter can enforce use of yield from in @contextmanager.

@contextmanager
def func():
    return other_generator_function()

A linter could ask you to change to:

@contextmanager
def func():
    yield from other_generator_function()

You may need an empty return if you have something that does branching and you need to short circuit.

JelleZijlstra, AlexWaygood, and Hnasar reacted with thumbs up emoji

asottile, jkittner, 93578237, graingert, and berkanteber reacted with thumbs down emoji

All reactions

👍 3 reactions
👎 5 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

hauntsaninja closed this

Sep 6, 2022

AlexWaygood removed the status: needs decision Needs a final decision by the typeshed maintainers label

Sep 7, 2022

asottile deleted the contextmanager-needs-generator branch

September 13, 2022 15:56

asottile mentioned this pull request

Sep 13, 2022

improve lru_cache by using ParamSpec #7771

Closed

flaeppe mentioned this pull request

Jun 22, 2023

Add memcache_key_warnings() typeddjango/django-stubs#1562

Merged

AlexWaygood mentioned this pull request

Sep 23, 2023

Fix contextlib.asynccontextmanager to work with coroutine functions #10753

Closed

AlexWaygood mentioned this pull request

Nov 1, 2023

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

Closed

Sachaa-Thanasius mentioned this pull request

Jun 2, 2024

Fix the signatures of @(async)contextmanager #12087

Merged

vvlrff mentioned this pull request

Aug 17, 2026

refactor: annotate contextmanager returns as Generator, not Iterator ag2ai/ag2#3176

Merged

5 of 6 tasks

Reviewers

srittau

srittau approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

asottile commented Mar 3, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

asottile commented Mar 3, 2022

Uh oh!

AlexWaygood commented Mar 3, 2022

Uh oh!

asottile commented Mar 3, 2022

Uh oh!

srittau commented Mar 3, 2022

Uh oh!

JelleZijlstra commented Mar 3, 2022

Uh oh!

AlexWaygood commented Mar 3, 2022

Uh oh!

asottile commented Mar 3, 2022

Uh oh!

hauntsaninja commented Mar 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Mar 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JelleZijlstra commented Mar 3, 2022

Uh oh!

hauntsaninja commented Mar 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asottile commented Mar 3, 2022

Uh oh!

AlexWaygood commented Mar 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Akuli commented Mar 4, 2022

Uh oh!

srittau commented Mar 4, 2022

Uh oh!

Akuli commented Mar 4, 2022

Uh oh!

srittau commented Mar 4, 2022

Uh oh!

hauntsaninja commented Mar 21, 2022

Uh oh!

asottile commented Mar 21, 2022

Uh oh!

asottile commented Mar 21, 2022

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

asottile commented Mar 25, 2022

Uh oh!

JukkaL commented Mar 28, 2022

Uh oh!

asottile commented Mar 28, 2022

Uh oh!

JukkaL commented Mar 28, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

jonathanslenders commented Jun 16, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

github-actions Bot commented Sep 4, 2022

Uh oh!

srittau left a comment

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra commented Sep 5, 2022

Uh oh!

AlexWaygood commented Sep 5, 2022

Uh oh!

twoertwein commented Sep 6, 2022

Uh oh!

hauntsaninja commented Sep 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

hauntsaninja commented Mar 3, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood commented Mar 3, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

hauntsaninja commented Mar 3, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

AlexWaygood commented Mar 4, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

hauntsaninja commented Sep 6, 2022 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.