Implement async generators (PEP 525) by JelleZijlstra · Pull Request #2711 · python/mypy

JelleZijlstra · 2017-01-19T04:58:28Z

This turned out to be pretty straightforward.

Two issues that may need more discussion:

3.6.0 was released without typing.AsyncGenerator; it was added in add AsyncGenerator typing#346 and should be in 3.6.1. But this patch assumes that typing.AsyncGenerator exists, so if you run mypy in 3.6.0 on a file with an async generator, it says that typing.AsyncGenerator doesn't exist. I originally put AsyncGenerator in mypy_extensions to work around this, but maybe we need some other solution.
PEP 525 says that it's a RuntimeError to yield inside a finally block in an async generator, but that doesn't appear to be true for 3.6.0 as implemented, so I didn't add a check in mypy for this.

This closes #2616

gvanrossum · 2017-01-19T20:21:16Z

Re: the absence of typing.AsyncGenerator in Python 3.6.0, I don't understand why that would cause an internal error in mypy, since mypy doesn't look up types needed for type checking the target program in the runtime typing.py module -- it looks in typeshed. Assuming that what you're really seeing is what happens when typeshed doesn't have typing.AsyncGenerator (assuming mypy --python-version 3.6 is used), we could just guard for that by syncing the typeshed submodule past the commit that has it. Or you could check whether the lookup failed and report an error instead.
I don't quite follow your second bullet (maybe you can show a complete test program with expected and actual behavior?) but @1st1 should be able to help you understand whatever it is you are seeing.

1st1 · 2017-01-19T20:26:08Z

PEP 525 says that it's a RuntimeError to yield inside a finally block in an async generator, but that doesn't appear to be true for 3.6.0 as implemented, so I didn't add a check in mypy for this.

It's a RuntimeError when you yield inside a finally block during AG finalization, i.e. when its .aclose() method is called or when it is GCed. An exact same rule exists for regular synchronous generators.

1st1 · 2017-01-19T20:27:35Z

IOW it's OK to have yield expressions in finally blocks in asynchronous generators, and I don't think mypy should care about that.

JelleZijlstra · 2017-01-19T21:09:00Z

Thanks for clarifying the yield/finally issue! An earlier version of my patch did have an internal error when typing.AsyncGenerator didn't exist, but that is fixed now (I amended my message above yesterday to reflect that). People using 3.6.0 will just have to guard their from typing import AsyncGenerator in if TYPE_CHECKING:.

In other words, both of the issues I flagged should be resolved.

ilevkivskyi · 2017-02-19T16:07:47Z

@JelleZijlstra What is the status of this PR? I am asking just to see whether it would be possible to have full Python 3.6 support in 0.480 (that should be released in around one week if I understand correctly the current schedule).

JelleZijlstra · 2017-02-19T16:33:02Z

@ilevkivskyi I think the code is ready, but it hasn't been reviewed yet. Also, there is a merge conflict now—I'll fix that.

ilevkivskyi

This mostly looks good, here are some minor comments.

ilevkivskyi · 2017-03-18T15:42:13Z

            return AnyType()
-        elif not self.is_generator_return_type(return_type, is_coroutine):
+        elif (not self.is_generator_return_type(return_type, is_coroutine)
+                and not self.is_async_generator_return_type(return_type)):


I can't comment there, but you might update the comment after elif return_type.args: below (add AsyncGenerator).

ilevkivskyi · 2017-03-18T16:04:18Z

                    return

-                if self.is_unusable_type(return_type):
+                if self.is_unusable_type(return_type) or defn.is_async_generator:


I would prefer a separate (more clear) error message for async generators, like: "return statement with value in async generator is prohibited". Maybe it could be even blocking (since formally speaking this is a SyntaxError).

Also, I would probably move this check to semanal.py.

I think putting this in semanal.py would be hard because we only discover that the function is a generator during semantic analysis. I suppose we could do it in ThirdPass, but there are some code comments suggesting that ThirdPass may be removed in the future.

(But I'll change the error message.)

ilevkivskyi · 2017-03-18T16:07:42Z

+
+async def gen() -> AsyncGenerator[int, None]:
+    yield 1
+    return 42  # E: No return value expected


I would add more tests here, both return None and return f() (for f that returns None) should fail (both with and without --strict-optional), since these are syntax errors.

ilevkivskyi · 2017-03-18T16:08:39Z

+-- ---------------------------------------------------------------------
+
+[case testAsyncGenerator]
+# flags: --fast-parser --python-version 3.6


--fast-parser flag is not necessary anymore.

ilevkivskyi · 2017-03-18T16:12:07Z

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


This is not important, but it looks a bit strange that you need to set both flags here.

ilevkivskyi · 2017-03-18T16:22:00Z

+async def f() -> AsyncGenerator[int, None]:
+    pass
+
+async def gen() -> AsyncGenerator[int, None]:


Maybe add some tests with AsyncIterator[int]?

ilevkivskyi · 2017-03-18T16:26:19Z

+
+async def genfunc() -> AsyncGenerator[int, None]:
+    yield 1
+    yield 2


Maybe also add a test where you yield a wrong type?

JelleZijlstra · 2017-03-18T17:04:08Z

Thanks for the comments! I will have to rebase the PR because it now relies on an obsolete version of typed_ast.

…nc_generator_return_type

…urns a value

Probably not worth it

JukkaL

LGTM -- only one idea for an additional test case.

JukkaL · 2017-03-18T18:48:46Z

 INVALID_EXCEPTION_TYPE = 'Exception type must be derived from BaseException'
 INVALID_RETURN_TYPE_FOR_GENERATOR = \
    'The return type of a generator function should be "Generator" or one of its supertypes'
+INVALID_RETURN_TYPE_FOR_ASYNC_GENERATOR = \


There doesn't seem to be a test for this error message. Can you add one?

JukkaL · 2017-03-18T19:36:53Z

🎉

JelleZijlstra mentioned this pull request Jan 19, 2017

Fix return values of athrow and aclose. python/typeshed#845

Merged

JelleZijlstra changed the title ~~Implement async generators (PEP 525)~~ [WIP] Implement async generators (PEP 525) Jan 19, 2017

JelleZijlstra changed the title ~~[WIP] Implement async generators (PEP 525)~~ Implement async generators (PEP 525) Jan 19, 2017

JelleZijlstra mentioned this pull request Feb 6, 2017

Add ast36 python/typed_ast#29

Closed

ilevkivskyi reviewed Mar 18, 2017

View reviewed changes

JelleZijlstra added 10 commits March 18, 2017 10:06

asyncgen: progress

c741a97

rename is_coroutine_generator -> is_async_generator, implement is_asy…

4dc62b6

…nc_generator_return_type

working implementation

c64e7fc

fix return types of athrow and aclose, error when async generator ret…

0117379

…urns a value

use AsyncGenerator from typing

01b50de

remove AsyncGenerator from mypy_extensions

feaf051

Probably not worth it

fix lint and make it crash less often on typing without AsyncGenerator

2273ac8

address Ivan's comments

c255d84

fix tests

facc5ed

fix obsolete test

f193e42

JelleZijlstra force-pushed the asyncgen branch from 74277c9 to f193e42 Compare March 18, 2017 17:19

JukkaL reviewed Mar 18, 2017

View reviewed changes

add test suggested by Jukka

dc0206c

JukkaL merged commit baadb04 into python:master Mar 18, 2017

JelleZijlstra deleted the asyncgen branch May 4, 2017 04:07

Conversation

JelleZijlstra commented Jan 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gvanrossum commented Jan 19, 2017

Uh oh!

1st1 commented Jan 19, 2017

Uh oh!

1st1 commented Jan 19, 2017

Uh oh!

JelleZijlstra commented Jan 19, 2017

Uh oh!

ilevkivskyi commented Feb 19, 2017

Uh oh!

JelleZijlstra commented Feb 19, 2017

Uh oh!

ilevkivskyi left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JelleZijlstra commented Mar 18, 2017

Uh oh!

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JukkaL commented Mar 18, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JelleZijlstra commented Jan 19, 2017 •

edited

Loading