Await repl #10390
Await repl #10390
Conversation
|
This is super cool, @Carreau! |
|
I think I might be able to execute the outer function in a separate
namespace to avoid potential name conflicts and conditional clearing of
user_ns. I'd like to have a clean way of still going through AST
transformers. I'm thinking of setting a flag, extraction the inner ADT of
the async function and shove it back in before exec.
Both of these should also allow to set your own runner without string
manipulation.
That's delicate surgery in the core of IPython though.
…On Mar 9, 2017 19:24, "Min RK" ***@***.***> wrote:
This is super cool, @Carreau <https://github.com/Carreau>!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10390 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAUez3U5qUWH8CmiT7vYAL6UVyQtGWCUks5rkMJwgaJpZM4MY6O6>
.
|
Carreau
force-pushed
the
await-repl
branch
2 times, most recently
from
db26de0 to
6fd07ad
Compare
|
I'm thinking of protecting it behind a flag, (on by default), so that you can disable the heuristic. |
Carreau
force-pushed
the
await-repl
branch
2 times, most recently
from
59c304b to
2c8176d
Compare
|
Ok, now compatible with In [1]: async def child1():
...: print(" child 1 go to sleep")
...: await trio.sleep(1)
...: print("child 1 wakes up")
In [2]: async def child2():
...: print(" child 2 go to sleep")
...: await trio.sleep(2)
...: print("child 2 wakes up")
...:
In [3]: import trio
In [4]: print('parent start')
...: async with trio.open_nursery() as n:
...: n.spawn(child1)
...: n.spawn(child2)
...: print('joined')
parent start
child 2 go to sleep
child 1 go to sleep
child 1 wakes up
child 2 wakes up
joined |
Carreau
force-pushed
the
await-repl
branch
10 times, most recently
from
962eec7 to
35336eb
Compare
|
Nice @Carreau! I have taken a shot at implementing this in jp-babel using babel-plugin-top-level-await but no luck yet. If I get a chance, I will revisit this and share any findings... |
|
Did you look at how I did the transformation to run the code in the prototype asyncio magic, now adopted by @Gr1N? I haven't thought through the implications in detail, but pulling the function body out after transformation feels neater than pushing namespaces into functions. |
|
The problem is that for a library like trio, we really want a single call
to trio.run() wrapped around the whole cell, because each call basically
creates its own event loop. Consider a cell like
print("start:", trio.current_time())
async with trio.open_nursery() as nursery:
nursery.spawn(...)
There are two showstopper issues:
- trio.current_time() is syntactically synchronous, but it can only be used
inside a call to trio.run (it uses some thread local storage thing to fetch
the current run's clock object).
- if we transform the async with to something like
# highly simplified:
tmp = trio.open_nursery()
trio.run(tmp.__aenter__)
...
trio.run(tmp.__aexit__)
then we will create the nursery in one run loop, and clean it up in a
different run loop. This is not going to work out well, as __aexit__ sits
and waits for tasks that its loop isn't even running...
|
|
... On the other hand tho I just realized an obvious place where the
shoving-locals-in-and-out thing doesn't work: closures.
I think with the current implementation we have:
>> a = 1
>> def blah():
... print(a)
>> blah()
1
>> a = 2
>> blah() # should print 2
1
?
|
well that seem to work: @takluyver I didn't found it, but it's an approach I tried. The other problem with this approach is that your await call are now blocking (are each are in a loop) and my initial need to write this was to actually make use of the concurrency to fetch parallel URL in the REPL. I (with @njsmith help ) went a couple of other direction (rewriting codeobjects, trying to exec only the inner body of a function) all of them a dead end. |
At least JS have a default event loo, so that's solved :-) |
It should only block on |
Fair enough, I'm unsure how the AST should rewrite that though: There is no await (in the second block) and top-level I'm guessing we can push the rewriting further though, I would be happy to give it a try (unsure how to treat async-with, and then you need to care about async-for as well ? and other constructs ?. I would like to not bake-in asyncio though. and in the end we should try to push for supporting this upstream instead of the crazy hack we have now. |
|
Ah, right, I didn't consider |
I don't want to get into the business of implementing our own event loop (at least not right now). |
| Handler for asyncio autoawait | ||
| """ | ||
| import asyncio | ||
| return asyncio.get_event_loop().run_until_complete(coro) |
There was a problem hiding this comment.
What would happen if ipykernel started running on asyncio? Would this cause hangs?
Likely, but if we make ipykernel use asyncio, we can always change this handler and make a minor release. And maybe by that time python 3.7 is out and will support actual async exec. I'm going to guess there might also be weird cases when you use uvloop, or if you register a loop in a different thread to have actual async execution. But I doubt this is worth trying to tackle now. |
| # Compile to bytecode | ||
| try: | ||
| code_ast = compiler.ast_parse(cell, filename=cell_name) | ||
| if _should_be_async(cell) and self.autoawait: |
There was a problem hiding this comment.
I'd reverse these since autoawait is a bool and _should_be_async doesn't need to parse if the bool is False.
| A compiled code object, to be executed | ||
| result : ExecutionResult, optional | ||
| An object to store exceptions that occur during execution. | ||
| async : Bool (Experimental) |
There was a problem hiding this comment.
General question: with async the almost-a-keyword, is there any indication that async as a variable name should be discouraged?
There was a problem hiding this comment.
Good catch – the plan is indeed to make async into a real keyword in 3.7, so it shouldn't be used as a name.
| If the object is a fully qualified object name, attempt to import it and | ||
| set it as the runner, and activate autoawait.""" | ||
|
|
||
| param = parameter_s.strip() |
There was a problem hiding this comment.
mixed case: 'on' 'Off'. Presumably both should be lowercase.
| loop_runner_map ={ | ||
| 'asyncio':_asyncio_runner, | ||
| 'curio':_curio_runner, | ||
| 'trio':_trio_runner, |
There was a problem hiding this comment.
It seems a little odd that tornado, the only async runner that we actually use in IPython, isn't supported here. Is there a reason for that?
| self.shell.loop_runner = self.shell.user_ns[param] | ||
| self.shell.autoawait = True | ||
| return None | ||
|
|
| ip = get_ipython() | ||
| iprs = lambda x: ip.run_cell(dedent(x)) | ||
|
|
||
| if sys.version_info > (3,5): |
There was a problem hiding this comment.
Rather than making the test definitions themselves conditional, why not skip the tests?
| async def ___wrapper___(): | ||
| {usercode} | ||
| locals() | ||
| return None |
There was a problem hiding this comment.
Just happened to look at this again, and noticed that I think this should be
async def __wrapper__():
try:
{usercode}
finally:
locals()
|
Closing in favor of #11155 (at least for now) |
6 participants
Thanks to useful discussion with @njsmith and @minrk. cc @takluyver
As await is not valid at top level, and extracting ast, or other modification of
__code__don't work,when
SyntaxErrorand wraping inasync defis not syntax error use that.Send
globals()into the function, run the function, extractlocals(), and updateuser_ns