Skip to content

bpo-39562: Allow executing asynchronous comprehensions in the asyncio REPL - #18968

Merged
pablogsal merged 5 commits into
python:masterfrom
isidentical:bpo-39562
Mar 19, 2020
Merged

bpo-39562: Allow executing asynchronous comprehensions in the asyncio REPL#18968
pablogsal merged 5 commits into
python:masterfrom
isidentical:bpo-39562

Conversation

@isidentical

@isidentical isidentical commented Mar 12, 2020 •
edited by pablogsal
Loading

Copy link
Copy Markdown
Member

https://bugs.python.org/issue39562

Allow executing asynchronous comprehensions on the top level when the
PyCF_ALLOW_TOP_LEVEL_AWAIT flag is given.
@Jackenmen

Jackenmen commented Mar 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Does this also fix the async comprehension with await that was mentioned in the issue?
e.g.

[await asyncio.sleep(1, x) for x in range(5)]

I just noticed there's no test added for that and I don't know if these two cases differ under the hood.
@isidentical

isidentical commented Mar 12, 2020

Copy link
Copy Markdown
Member Author

Yes, it is! Will add a test.

>>> import asyncio
>>> [await asyncio.sleep(1, x) for x in range(5)]
[0, 1, 2, 3, 4]
@Jackenmen

Jackenmen commented Mar 12, 2020

Copy link
Copy Markdown
Contributor

Thanks for checking that :)
@isidentical
isidentical requested review from 1st1 and pablogsal March 12, 2020 19:50
@isidentical

isidentical commented Mar 12, 2020

Copy link
Copy Markdown
Member Author

I'm not sure if this is an enhancement or a bugfix so should we backports this to the 3.8? @1st1 @pablogsal
@pablogsal

pablogsal commented Mar 12, 2020

Copy link
Copy Markdown
Member

I think is a bug, this should be allowed the same way regular for loops are allowed so we need to backport this.
pablogsal
pablogsal reviewed Mar 12, 2020
View reviewed changes
Comment thread Lib/test/test_builtin.py Outdated
pablogsal
pablogsal reviewed Mar 12, 2020
View reviewed changes
Comment thread Python/compile.c Outdated
pablogsal
pablogsal reviewed Mar 12, 2020
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2020-03-12-22-13-50.bpo-39562.E2u273.rst Outdated
@isidentical
isidentical requested a review from pablogsal March 13, 2020 08:51
@pablogsal pablogsal changed the title bpo-39562: Run asynchronous comprehensions under asyncio repl Mar 15, 2020
@pablogsal pablogsal self-assigned this Mar 15, 2020
@pablogsal
pablogsal merged commit 9052f7a into python:master Mar 19, 2020
@miss-islington

miss-islington commented Mar 19, 2020

Copy link
Copy Markdown
Contributor

Thanks @isidentical for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 19, 2020
@isidentical @miss-islington
bpo-39562: Allow executing asynchronous comprehensions in the asyncio…
80a0928 
… REPL (pythonGH-18968)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
(cherry picked from commit 9052f7a)

Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com>
@bedevere-bot

bedevere-bot commented Mar 19, 2020

Copy link
Copy Markdown

GH-19070 is a backport of this pull request to the 3.8 branch.
miss-islington added a commit that referenced this pull request Mar 19, 2020
@miss-islington @isidentical
bpo-39562: Allow executing asynchronous comprehensions in the asyncio…
ec8a973 
… REPL (GH-18968)

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
(cherry picked from commit 9052f7a)

Co-authored-by: Batuhan Taşkaya <47358913+isidentical@users.noreply.github.com>
Carreau
Carreau reviewed Jul 6, 2020
View reviewed changes
Comment thread Python/compile.c

outermost = (comprehension_ty) asdl_seq_GET(generators, 0);
if (IS_TOP_LEVEL_AWAIT(c)) {
c->u->u_ste->ste_coroutine = 1;

@Carreau Carreau Jul 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this was over aggressive, it marks even non-async list comprehensions as coroutine, which breaks IPython/Jupyter. It should be marked as ste_coroutine only if await is somewhere.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

7 participants

@isidentical @Jackenmen @pablogsal @miss-islington @bedevere-bot @Carreau @the-knights-who-say-ni