Skip to content

Tweak pre-3.8 iscoroutine stub #8104

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

Merged

Conversation

florimondmanca
Copy link
Contributor

@florimondmanca florimondmanca commented Jun 19, 2022

Refs #6105 (comment)

This PR updates the stub for asyncio.iscoroutine(coro).

#6105 chose to define the return type as Union[GeneratorType, Coroutine] on Python pre-3.8. The rationale was that generator-style coroutine functions (@asyncio.coroutine()) were deprecated in Python 3.8, so developers running in previous versions of Python (Python 3.7 is the only non-EOL example) needed to know that what they get might not be an actual Coroutine.

asyncio does have GeneratorType as a possible _COROUTINE_TYPE:

_COROUTINE_TYPES = (types.CoroutineType, types.GeneratorType,
                    collections.abc.Coroutine, CoroWrapper)

So at runtime, asyncio.iscoroutine(coro) is True if coro comes from an @asyncio.coroutine(), i.e. functions decorated like this do return GeneratorType instances.

But type checkers don't seem to be aware of this. In fact, at check time, coro is shown as an AwaitableGenerator, not a Generator. This AwaitableGenerator is a special type defined in typeshed specifically for this use case:

typeshed/stdlib/typing.pyi

Lines 392 to 396 in 1be5918

# NOTE: This type does not exist in typing.py or PEP 484 but mypy needs it to exist.
# The parameters correspond to Generator, but the 4th is the original type.
class AwaitableGenerator(
Awaitable[_V_co], Generator[_T_co, _T_contra, _V_co], Generic[_T_co, _T_contra, _V_co, _S], metaclass=ABCMeta
): ...

(It is marked for displacement from typing to _typeshed, here: #7580. I'm using the current location here.)

By using GeneratorType in the stub, #6105 made it impossible to use the type guard before an await coro on Python pre-3.8 (i.e. 3.7), because mypy raises an error saying that generators can't be awaited.

To reproduce, run mypy --python-version 3.7 on this script:

import asyncio
from typing import Callable

def await_me_maybe(func: Callable):
    obj = func()

    if asyncio.iscoroutine(obj):
        obj = await obj  # XXX

    return obj

mypy output:

error: Incompatible types in "await" (actual type "Union[GeneratorType[Any, Any, Any], Coroutine[Any, Any, Any]]", expected type "Awaitable[Any]")  [misc]

This PR switches to AwaitableGenerator which seems to be the proper check-time type.

I originally stumbled upon this in: encode/httpx#2269

@github-actions

This comment has been minimized.

@JelleZijlstra
Copy link
Member

I don't think we should do this. AwaitableGenerator is a mypy-specific name that doesn't actually exist, and we shouldn't have it in typing in the first place.

@hauntsaninja
Copy link
Collaborator

There is a real usability issue here though. I wouldn't mind lying here and having Annotated[TypeGuard[Coroutine[Any, Any, Any]], "can actually be a generator-style coroutine"] for 3.7 as well.

@florimondmanca florimondmanca force-pushed the fm/iscoroutine-awaitablegenerator branch from 6b1eed2 to 9eff23b Compare June 28, 2022 09:30
@github-actions

This comment has been minimized.

@florimondmanca
Copy link
Contributor Author

Hi, I updated the PR with the Annotated suggestion. Please let me know if that seems acceptable. In the end mypy treats iscoroutine as returning the same TypeGuard[Coroutine[Any, Any, Any]], so the issue I mentioned would be resolved, but we do keep a note that it might be an @asyncio.coroutine.

def iscoroutine(obj: object) -> TypeGuard[types.GeneratorType[Any, Any, Any] | Coroutine[Any, Any, Any]]: ...
def iscoroutine(
obj: object,
) -> Annotated[TypeGuard[Coroutine[Any, Any, Any]], "can actually be a generator-style coroutine"]: ...
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to have a comment instead of using Annotated here, in the off-chance this interacts poorly with someone else's use of Annotated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy with that too.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Sounds like we're all happy with this now. Thanks for the PR!

@AlexWaygood AlexWaygood merged commit 2792910 into python:master Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants