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

Unexpected behavior regarding async generator #5070

Closed
pawelswiecki opened this issue May 17, 2018 · 9 comments
Closed

Unexpected behavior regarding async generator #5070

pawelswiecki opened this issue May 17, 2018 · 9 comments

Comments

@pawelswiecki
Copy link

pawelswiecki commented May 17, 2018

Trying to get typing work in an asynchronous code and I don't understand mypy's behavior.

Let's say we have two coroutines typed in the following way:

from typing import AsyncGenerator

async def gen1() -> AsyncGenerator[str, None]:
    pass
reveal_type(gen1)

async def gen2() -> AsyncGenerator[str, None]:
    yield 'tick'
reveal_type(gen2)

mypy:

$ mypy file.py
file.py:5: error: Revealed type is 'def () -> typing.Awaitable[typing.AsyncGenerator[builtins.str, builtins.None]]'
file.py:9: error: Revealed type is 'def () -> typing.AsyncGenerator[builtins.str, builtins.None]'

So the type of the first one gets wrapped in Awaitable. I don't fully understand why. I thought all Async... are Awaitables by definition. I did not find anything about it but here #3576, which is not enough for me to get it.

The same thing in case of methods:

class BaseClass:
    async def gen1(self) -> AsyncGenerator[str, None]:
        pass
reveal_type(BaseClass.gen1)

class MyClass(BaseClass):
    async def gen1(self) -> AsyncGenerator[str, None]:
        yield 'tick'
reveal_type(MyClass.gen1)

mypy:

file.py:4: error: Revealed type is 'def (self: acm.workers.BaseClass) -> typing.Awaitable[typing.AsyncGenerator[builtins.str, builtins.None]]'
file.py:7: error: Return type of "gen1" incompatible with supertype "BaseClass"
file.py:9: error: Revealed type is 'def (self: acm.workers.MyClass) -> typing.AsyncGenerator[builtins.str, builtins.None]'

So now I cannot define abstract methods that are async generators, in a straightforward way.

Versions

Python 3.6.5
mypy==0.600

EDIT: I fixed a random typo @JelleZijlstra found and edited parts of the text based on the typo. Now the whole problem is clearer.

@JelleZijlstra
Copy link
Member

Your gen1 is not a generator (it has no yield), so mypy interprets it as a normal async function that returns an AsyncGenerator object. gen2 is actually correctly typed as def () -> typing.AsyncGenerator[builtins.str, builtins.None], but your second reveal_type has a typo.

The issue with overrides can be explained by the same logic: the base class's gen1 is not a generator because there's no yield. You can probably work around it by adding a dummy yield inside the function or by removing the async from its def (I haven't tested), but it does seem like a usability issue that the obvious way to type an abstract AsyncGenerator doesn't work.

@ilevkivskyi
Copy link
Member

The if False: yield trick is as old as Python probably is. I think the source of confusion here may be that type annotations are completely ignored by Python runtime. Just by typing something as a generator it will not magically become a generator, you still need a yield somewhere.

There is unfortunately no simple solution, we may just document this in common issues.

@pawelswiecki
Copy link
Author

Thank you both, now I get it. My typo (which I will correct in a second) only added to my confusion. I agree that this probably should be explained in docs somewhere.

@pawelswiecki
Copy link
Author

Update: I started to use plain yield in those kind of abstract async generators and I can say I like it: it's just as succinct as pass (which is, in this context, equivalent to plain return) and informative for the user as well.

facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jan 30, 2019
Summary:
Came across this because we were wrapping functions returning `AsyncGenerator` inside an `Awaitable`, which was throwing false positive errors later on when we tried to call `__aiter__` on it, for instance.

Turns out functions prefixed with `async` wrap the return type in a `Coroutine`, not an `Awaitable` (see: python/mypy#3569), and functions that are actually generators (contain a yield) just take the return annotation of `AsyncGenerator` at face value - otherwise, the function signature is understood as asynchronously returning a generator object just like any other async function (see: python/mypy#5070)

Reviewed By: dkgi

Differential Revision: D13864544

fbshipit-source-id: 0d201735252b77688a5491428cfb5818d000754b
@hlovatt
Copy link

hlovatt commented May 4, 2020

I too like the plain yield but think it needs to be documented, it took a bit of googling to find this page!

@ashb
Copy link

ashb commented Feb 25, 2022

A slightly different case to this (that is still fixed by doing if False: yield) is if your parent ABC class:

class Base(abc.ABC):
    @abc.abstractmethod
    def run(self) -> AsyncIterator[int]:
        raise NotImplementedError("must implement run()")

class A(Base):
    def run(self) -> AsyncIterator[int]:
        while True:
            yield 1

This will fail with

test.py:34: error: Return type "AsyncIterator[int]" of "run" incompatible with return type "Coroutine[Any, Any, AsyncIterator[int]]" in supertype "Base" [override]

By making it a generator it passes

    @abc.abstractmethod
    def run(self) -> AsyncIterator[int]:
        raise NotImplementedError("must implement run()")
        if False:
            yield 0

Even if the raise NotImplementedError isn't in the base function body the error remains the same. Should this ABC case be handed specially?

@pamelafox
Copy link

Sharing a similar example as the previous comment:

approach.py:

from abc import ABC, abstractmethod
from typing import Any, AsyncGenerator


class ChatApproach(ABC):
    @abstractmethod
    async def run(self, history: list[dict[str, str]], overrides: dict[str, Any], should_stream: bool) -> AsyncGenerator[dict, None]:
        ...

chatreadretrieveread.py:

from approaches.approach import ChatApproach

class ChatReadRetrieveReadApproach(ChatApproach):

    async def run(self, history: list[dict[str, str]], overrides: dict[str, Any], should_stream: bool=False) -> AsyncGenerator[dict, None]:

results in:

approaches/chatreadretrieveread.py:60: error: Signature of "run" incompatible with supertype "ChatApproach"  [override]
approaches/chatreadretrieveread.py:60: note:      Superclass:
approaches/chatreadretrieveread.py:60: note:          def run(self, history: list[dict[str, str]], overrides: dict[str, Any], should_stream: bool) -> Coroutine[Any, Any, AsyncGenerator[dict[Any, Any], None]]
approaches/chatreadretrieveread.py:60: note:      Subclass:
approaches/chatreadretrieveread.py:60: note:          def run(self, history: list[dict[str, str]], overrides: dict[str, Any], should_stream: bool = ...) -> AsyncGenerator[dict[Any, Any], None]

The "if False: yield" trick does work to remove the mypy errors.

@hauntsaninja
Copy link
Collaborator

This has been documented at https://mypy.readthedocs.io/en/stable/more_types.html#asynchronous-iterators for a couple months now

@ikonst
Copy link
Contributor

ikonst commented Aug 16, 2023

Took a stab at adding contextual help in #15883.

(... Although, this wouldn't help #5070 (comment) since it's a difference in more than just the return type...)

hauntsaninja pushed a commit that referenced this issue Aug 30, 2023
For issue described in #5070 and documented in #14973, add a contextual
link to the docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants