Implement 'async def' and friends ('await', 'async for', 'async with') by gvanrossum · Pull Request #1808 · python/mypy · GitHub

This is not ready for merge, but I have a question. [UPDATE: it is ready!]

@JukkaL Why would these two functions not be equivalent?

from typing import *

T = TypeVar('T')

async def f(x: T) -> T:
    y = await f(x)
    reveal_type(y)
    return y

async def g(x: T) -> T:
    gx = g(x)
    y = await gx
    reveal_type(y)
    return y

In f(), the revealed type is Any, while in g() it is T (spelled as 'T-1'). I would expect it to be T` in both cases.

I suspect that the reason is the Any types in type context in visit_await_expr. The type context should be derived from the type context of the await expression, perhaps.

If the type context is X[Any], then mypy tries to infer type X[Any] for the expression, which is happening in the f() case.

Ensure 'async def' is a syntax error in Python 2 (at least with the fast parser).

Taking a look at this now!

This is a nit, but it'd be better if these were left in their original locations rather than separated out. Currently, the visit_* functions in this file are in the same order as the nodes in the Python.asdl, which is a nice property to maintain. Also, most of them are virtually identical to the non-async versions -- having them right next to each other will make it more likely they stay in sync.

That makes sense -- moved them back (except moved AsyncFunctionDef up so it's better paired with FunctionDef). Also did a bunch of work on checker.py to remove the remaining XXX comments (originating in this PR).

is_generator_return_type() now takes an extra is_coroutine flag.

Await is required to have an expr, right?

This revealed a spurious error "Function does not return a value", fixed that.

I thought they were both None?

Actually in the code below ts is Void, tr is Any. But I wonder if it isn't better to make both None -- forcing developers to declare the return type as Generator[ty, ts, tr] if they want to either receive or return values. There's a unit test that currently checks that return value is allowed in a generator declared as Iterator but I think I'll just change that too.

Do you need this check? Doesn't an Awaitable always have a type parameter?

IIUC when you use a generic class without parameters args is empty. I also worry about crashes when someone writes e.g. Generator[int] -- it may well produce an error message but also an Instance with len(args) == 1, so I'm doing this defensively.

That seems reasonable. We should probably consider filling in the parameters at some earlier stage so we can later rely on the length...

… regular. Also Use get_generator_return_type() instead of manually unpacking the value.

Pushed responses to most of the review.

I actually still haven't figured out how to extract the type parameter from a subclass of Awaitable[T](if it's even possible).

I also found that the error message you get when the return type for __aenter__ or __aexit__ is wrong you get a pretty poor error message (just "Incompatible types"). I suspect it's much the same for async for. (But there's nice custom formatting code for the error coming out of await -- I probably need to refactor things to reuse more.)

@ddfisher that's all I planned to do in this PR -- another review please?

Do you still use this now that you have get_awaitable_return_type?

get_awaitable_return_type() is for getting the type of the expression in await, async for and async def. get_generator_return_type() is for the return statement inside a generator or async def.

Ah, I see. get_awaitable_return_type shouldn't have a parallel name, then. It'd also be nice to mention that in the docstrings -- those are also virtually identical at the moment.

The Awaitable parts of get_generator_yield_type and get_generator_recieve_type are never used anywhere. Are they necessary? With that in mind, maybe it makes sense to split out the Awaitable part of get_generator_return_type?

Can we not make this change as part of this diff?

OK.

Finished with review!

… Any.

Just one remaining nit around the naming/documentation of get_awaitable_return_type.

…docstring.

OK, done.

Thanks! Now I'll start with the follow-on PR...

gvanrossum force-pushed the async-await branch from 483acc9 to ef446ea Compare July 6, 2016 21:51

gvanrossum force-pushed the async-await branch 2 times, most recently from 952551d to 0e6634d Compare July 15, 2016 16:46

gvanrossum mentioned this pull request Jul 16, 2016

Design for using async+await with asyncio #1886

Closed

Guido van Rossum added 11 commits July 19, 2016 15:43

PEP 492 syntax: async def and await.

381ec52

Fix type errors; add async for and async with (not fully fledged).

be91786

Dispose of Async{For,With}Stmt -- use is_async flag instead.

cf3b9e0

Basic async for is working.

c8ea54b

Clear unneeded TODOs.

a8e9d7e

Fledgeling async with support.

41c5ed0

Disallow yield [from] in async def.

006d4b7

Check Python version before looking up typing.Awaitable.

eba9f0c

Ensure 'async def' is a syntax error in Python 2 (at least with the fast parser).

Vast strides in accuracy for visit_await_expr().

217116a

Add @with_line to PEP 492 visit function definitions.

f294d81

Fix tests now that errors have line numbers.

9e2a2eb

Tweak tests for async/await a bit.

c077d32

gvanrossum force-pushed the async-await branch from 0e6634d to c077d32 Compare July 19, 2016 23:23

ddfisher reviewed Jul 20, 2016
View reviewed changes

Guido van Rossum added 2 commits July 19, 2016 17:22

Get rid of remaining XXX issues.

c8d1cda

is_generator_return_type() now takes an extra is_coroutine flag.

Move PEP 492 nodes back where they belong.

b5b154b

ddfisher reviewed Jul 20, 2016
View reviewed changes

Guido van Rossum added 4 commits July 19, 2016 19:12

Respond to code review.

1d14cca

Add tests expecting errors from async for/with.

b21aae9

Test that await <generator> is an error.

3a766cb

Verify that yield from does not accept coroutines.

0746ade

This revealed a spurious error "Function does not return a value", fixed that.

ddfisher reviewed Jul 20, 2016
View reviewed changes

Guido van Rossum added 3 commits July 20, 2016 15:23

Disallow return value in generator declared as -> Iterator.

ebd9de5

Fix typo in comment.

3b687c5

Refactor visit_with_stmt() into separate helper methods for async and…

1bab7e0

… regular. Also Use get_generator_return_type() instead of manually unpacking the value.

Guido van Rossum added 4 commits July 20, 2016 21:29

Fix lint error. Correct comment about default ts/tr.

adc32a2

Improve errors when __aenter__/__aexit__ are not async. With tests.

649386e

Refactor: move all extraction of T from Awaitable[T] to a single helper.

607621c

Follow __await__ to extract t from subclass of Awaitable[t].

c79898b

ddfisher reviewed Jul 22, 2016
View reviewed changes

Make get_generator_return_type() default to AnyType() (i.e. as it was).

2e9a6a5

gvanrossum mentioned this pull request Jul 23, 2016

Allow yielding result of function that returns None #1933

Open

Fix test to match reverting get_generator_return_type() to default to…

e32a914

… Any.

Rename get_awaitable_return_type() to check_awaitable_expr(), update …

6581ce7

…docstring.

ddfisher merged commit dcc3b86 into master Jul 27, 2016

ddfisher deleted the async-await branch July 27, 2016 00:35

gvanrossum mentioned this pull request Jul 27, 2016

Support PEP 492 syntax #1453

Closed

+                              self.fail("'yield' in async function", expr, True, blocker=True)
+                          else:
+                              self.function_stack[-1].is_generator = True
+                      if expr.expr:

+                      elif return_type.type.fullname() == 'typing.Awaitable':
+                          # Awaitable is the other type which specifies the type of values it returns into
+                          # `yield from` expressions (using `return`).
+                          if len(return_type.args) == 1:

+                              return return_type.args[2]
+                          else:
+                              return AnyType()
+                      elif return_type.type.fullname() == 'typing.Awaitable':

                       else:
                           # `return_type` is supertype of Generator, so callers won't be able to see the return
                           # type when used in a `yield from` expression.
+                          return Void()

                   def visit_Index(self, n: ast35.Index) -> Node:
                       return self.visit(n.value)
+                  # PEP 492 nodes: 'async def', 'await', 'async for', 'async with'.

+                  #
+                  # A classic generator must define a return type that's either
+                  # `Generator[ty, ts, tr]`, Iterator[ty], or Iterable[ty] (or
+                  # object or Any).  If ts/tr are not given, both are Any.

Implement 'async def' and friends ('await', 'async for', 'async with') by gvanrossum · Pull Request #1808 · python/mypy · GitHub

Repository navigation

Implement 'async def' and friends ('await', 'async for', 'async with') - #1808

Implement 'async def' and friends ('await', 'async for', 'async with')#1808ddfisher merged 28 commits intomasterpython/mypy:masterfrom async-awaitpython/mypy:async-awaitCopy head branch name to clipboard

Conversation

gvanrossum commented Jul 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JukkaL commented Jul 6, 2016

Uh oh!

ddfisher commented Jul 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum Jul 20, 2016 via email

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddfisher Jul 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gvanrossum commented Jul 21, 2016

Uh oh!

gvanrossum commented Jul 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddfisher commented Jul 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddfisher commented Jul 22, 2016

Uh oh!

ddfisher commented Jul 26, 2016

Uh oh!

gvanrossum commented Jul 27, 2016 via email

Uh oh!

gvanrossum commented Jul 27, 2016 via email

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Implement 'async def' and friends ('await', 'async for', 'async with')#1808
ddfisher merged 28 commits into
masterfrom
async-await

gvanrossum commented Jul 5, 2016 •
edited
Loading

ddfisher Jul 20, 2016 •
edited
Loading