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

Question regarding Coroutine vs Awaitable #4460

Closed
ojii opened this issue Jan 12, 2018 · 3 comments
Closed

Question regarding Coroutine vs Awaitable #4460

ojii opened this issue Jan 12, 2018 · 3 comments
Labels

Comments

@ojii
Copy link

ojii commented Jan 12, 2018

Given this code:

from typing import *
from collections import deque

Task = Coroutine[Any, None, Any]
tasks: Deque[Task] = deque()


async def foo() -> None:
    print('foo!')


def add_task(task: Task) -> None:
    tasks.append(task)


def run() -> None:
    task = tasks.popleft()
    try:
        task.send(None)
    except StopIteration:
        pass


add_task(foo())
run()

Versions:

$ mypy --version
mypy 0.560-51e044c4ecf2a52fc6c41ee63019723e0d3061e1-dirty
$ python --version
Python 3.6.4

Running mypy --strict script.py I get script.py:24: error: Argument 1 to "add_task" has incompatible type "Awaitable[None]"; expected "Coroutine[Any, None, Any]", but changing Task to Awaitable[None] gives me script.py:19: error: "Awaitable[None]" has no attribute "send". The code runs fine though.

What would be the proper type hints in this scenario?

@JelleZijlstra
Copy link
Member

I think this is a bug in mypy. Adding reveal_type(foo()) to your example gives bin/coro.py:24: error: Revealed type is 'typing.Awaitable[builtins.None]', which I think is wrong; foo() returns a Coroutine.

This might be fairly easy to fix—if you are able to, feel free to submit a PR to fix the issue.

@ethanhs ethanhs added bug mypy got something wrong priority-1-normal size-small labels Jan 12, 2018
@ojii
Copy link
Author

ojii commented Jan 12, 2018

This might be fairly easy to fix

The tricky thing is that Coroutine takes 3 arguments (like Generator) but Awaitable only one. How would the send and yield types of Coroutine be filled?

Edit: I tried to have a look around the codebase and see if I can figure out what would need changing but it's all a bit too complicated for me, so unfortunately I think I cannot fix this myself at the moment.

@AndreLouisCaron
Copy link
Contributor

This problem is similar to #3569. I ran the fix in #5052 against this script and the error disappears.

ilevkivskyi pushed a commit that referenced this issue May 17, 2018
* Change the return type of coroutine functions to coroutine object

Previously, the return type was `Awaitable`, which is correct, but
not specific enough for some use cases.  For example, if you have
a function parameter that should be a coroutine object (but not a
`Future`, `Task` or other awaitable), then `mypy` is unable to
detect incorrect invocations of the function.

This change is (deliberately) imcomplete as is.  It seems like
this breaks quite a few tests in `mypy`.  The first symptom is that
coroutine objects are now (incorrectly) detected as generators..

* Prevent coroutine functions from being classified as generators

This change removes the undesired "return type of a generator
function should be `Generator` or one of its subtypes for coroutine
function definitions.

However, it introduces a new error where the return type of coroutine
functions is expected to be `Coroutine[T, Any, Any]` instead of the
desired `T`.  It looks like we were hijacking a generator-specific
path for checking the return type of coroutine functinos.  I added
an explicit path for coroutine functions, allowing our test to pass.

However, lots of tests now fail.  A few of them were simply places
that were incidentally relying on coroutine functions to have type
`Awaitable`.  I fix them.  The remaining failures all seem to be
about coroutine functions with return type `None` without an explicit
return statement.  Seems like this is also something for which we
were relying on implicit classification as generators.

* Allow implicit return for coroutine functions that return `None`

Most of the tests are fixed, but two tests still fail.  One about
not detecting invalid `yield from` on `AwaitableGenerator`.  The
other about types being erased in call to `asyncio.gather()`.

* Fix return type for coroutine functions decorated with @coroutine

* Fix detection of await expression on direct coroutine function call

Changing the return type of coroutine functions to  `Coroutine`
introduced a regression in the expression checks.

* Fix regression after change of coroutine function return type

* Fix position of return type in `Coroutine`

This fixes the type inference logic that was causing the last
failing test to fail.  Build should now be green :-)

* Fix issues raised in code review

Fixes #3569.
Fixes #4460.

Special thanks to @ilevkivskyi and @gvanrossum for their time
and their infinite patience with all my questions :-)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants