add TypeGuard to coroutines.iscoroutine

[KotlinIsland]

No description provided.

[KotlinIsland]

Would it be possible if I could get some Hacktoberfest brownie points?
If you don't mind adding the hacktoberfest-accepted label 😀

[srittau]

I believe we should try just returning TypeGuard[Coroutine[Any, Any, Any]] here. This will hopefully resolve the primer problems. Generator-based coroutines are deprecated/not supported anymore anyway, but if we were to supported them, we'd need to use collections.abc.Generator[Any, Any, Any], instead of GeneratorType.

[KotlinIsland]

I believe GeneratorType is correct here, as the actual implementation uses GeneratorType to do the check.

from asyncio import iscoroutine
from collections.abc import Generator


class MyGenerator(Generator):

    def send(self, __value):
        pass

    def throw(self, __typ, __val = ..., __tb = ...):
        pass

def foo(): yield 1

print(iscoroutine(MyGenerator()))  # False
print(iscoroutine(foo()))  # True

[KotlinIsland]

The entire use case of asyncio.iscoroutine compared to inspect.iscoroutine is that it checks for generators. so mistyping it to please primer sounds like the wrong way to go.

[srittau]

There are two options here, per our recommendations to avoid union return types: Use just Coroutine or keep returning bool. In this case, considering the deprecation of generator coroutines, using TypeGuard[Coroutine] seems like the better option. Primer output actually proves that using a union return type is not serving our users well.

[KotlinIsland]

I get what you're saying, but I think given the close relationship between asyncio.iscoroutine and inspect.iscoroutine, where the latter checks for just async coroutines and the former checks for all types, I think it would be a mistake to type this as just Coroutine, I think it would lead to confusing edge cases where generator style coroutines aren't expected and the developer should have been using inspect.iscoroutine.

[KotlinIsland]

Double actually, I think it should narrow to GeneratorType, because of:

from asyncio import iscoroutine, run

def do_something1(x: object):
    if iscoroutine(x):
        x.cr_await

async def do_something2(x: object):
    if iscoroutine(x):
        await x

do_something1(_ for _ in ())
run(do_something2(_ for _ in ()))

This has nothing at all to do with using deprecated @coroutines and is just generic code that will now have runtime errors but no type errors.

[KotlinIsland]

Although if you ask me the implementation of iscoroutine should be updated.

[florimondmanca]

As a note, in encode/httpx#2269 where we have this kind of usage:

response = func_maybe_async()

if asyncio.iscoroutine(response):
    response = await response

We had to narrow down to TypeGuard[Coroutine] ourselves because on py3.7 mypy would raise an error due to Generator not being awaitable.

It turns out that mypy types the result of coroutine functions defined as @asyncio.coroutine() as AwaitableGenerator which mypy is OK to await. Running with check_untyped_defs = True:

import asyncio

@asyncio.coroutine
def wait():
    yield from asyncio.sleep(1)

async def main():
    coro = wait()
    reveal_type(coro)  # AwaitableGenerator[Any, Any, Any, Any]
    await coro  # OK

So it's actually impossible to get a coroutine that would be a Generator at runtime (generators can't be await-ed).

So maybe the correct pre-3.8 annotation would have been Union[AwaitableGenerator, Coroutine]?

[KotlinIsland]

Thanks, raised it here #8114

[KotlinIsland]

Actually it's here #8104

[github-actions]

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

core (https://github.com/home-assistant/core.git)
+ homeassistant/helpers/service.py:703: error: unused "type: ignore" comment

aiohttp (https://github.com/aio-libs/aiohttp.git)
+ aiohttp/web.py:307: error: unused "type: ignore" comment

[srittau]

Thanks!