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

py/compile: Do not await __aiter__ special method return value. #6272

Closed
wants to merge 2 commits into from

Conversation

jonathanhogg
Copy link
Contributor

As per discussion in #6267, the final PEP492 spec says that the __aiter__() method should not return an awaitable. This PR changes the compiler to not compile in an await and updates the async for tests to match.

Note that the test in async_for2.py has had to be more modified in behaviour as well as syntax as it can no longer yield in the __aiter__() method. I've just removed the call to await f() and the matching output completely.

Have run the UNIX port tests successfully on macOS and also tested with my own async iterable code on the ESP32 port.

Note: This will be a breaking change to any (not precompiled) code that currently defines __aiter__ methods using await.

__aiter__ should return an async-iterable object, but is not itself 
awaitable - as per CPython documentation. See micropython#6267.
Remove async keyword from __aiter__ methods to match 
CPython/PEP452-final spec. Also remove await of f() from __aiter__ 
method in async_for2.py as this is no longer valid (updated expected 
output to match).
@peterhinch
Copy link
Contributor

The transition to uasyncio V3 introduces other breaking changes in the interests of CPython compatibility, so in my view now is the time to do this.

@peterhinch
Copy link
Contributor

CPython's syntax rules have changed. V3.6.9 accepts either

    async def __aiter__(self):
        print('aiter')
        return self

or

    def __aiter__(self):
        print('aiter')
        return self

CPython 3.8.0 accepts only the latter, throwing a cryptic exception in the former case.

@jonathanhogg
Copy link
Contributor Author

CPython's syntax rules have changed. V3.6.9 accepts either
[...]
CPython 3.8.0 accepts only the latter, throwing a cryptic exception in the former case.

I think this is consistent with this message on the original CPython bug, which suggested that the change be staggered in with a PendingDeprecationWarning in 3.5.2, a DeprecationWarning in 3.6 and then dropping support for async __aiter__() in 3.7.

I've just tested on a 3.7 build and using async fails:

  File "aiter_test2.py", line 26, in main
    async for i in SlowCounter(10):
TypeError: 'async for' received an object from __aiter__ that does not implement __anext__: coroutine

@dpgeorge
Copy link
Member

The transition to uasyncio V3 introduces other breaking changes in the interests of CPython compatibility, so in my view now is the time to do this.

I was thinking to wait until after v1.13 is released to change/fix this, but, I think you're right, let's do it now.

@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Jul 24, 2020
@dpgeorge
Copy link
Member

Rebased, squashed (in this case it's good to have the code change go with the change to the tests) and merged in 37e1b5c

Thank you!

@dpgeorge dpgeorge closed this Jul 24, 2020
@jonathanhogg jonathanhogg deleted the aiter_fix branch July 24, 2020 16:49
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request May 12, 2022
…t-fix

Force pin 21 high; patch; need to diagnose further
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants