Add a trio repl #2972
Add a trio repl #2972
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2972 +/- ##
========================================
Coverage 99.64% 99.64%
========================================
Files 117 120 +3
Lines 17594 17711 +117
Branches 3171 3177 +6
========================================
+ Hits 17532 17649 +117
Misses 43 43
Partials 19 19
|
|
I am in favor of accepting something like this. As the github bot has noted, please add type hints. Does Ctrl+C work reliably to cancel the running thing? Does Ctrl+C at the REPL prompt just give you another prompt rather than crashing the REPL? I would say that both of those are important usability features, and you might have them without trying but it's worth a test. |
|
Also would it be worth it to have a nursery created by default in a (Not sure how cancellation would work there though. So half-baked idea... probably best as a service nursery or whatever? But we don't have those in trio!!) ... actually thinking a bit more, I can't really think of a use case. |
|
I like this pull request a lot more than #2407 |
Yes, I have start on that. Just a few issues I need to come up with a fix for.
As far as I can tell, yes, Ctrl+C handling is robust and yes, it keeps you in the repl. The robustness comes from trio to begin with!
This is actually what initially motivated me to start down this path, but I hadn't worked out how I was going to do it. So thanks for the suggestion. I will have a play with that idea after getting the basics sorted out. Thanks for the positive feedback. It will probably take a day or two to tidy up the type hinting |
to save me+others some time comparing them if you already have - why? What's the difference between their approach? |
|
For the nursery, perhaps what it could do is that when control-C is pressed, first stop any foreground code that's running as normal. But if pressed again with nothing, cancel the nursery, close it, then replace it with a fresh nursery that's open. If a user causes the nursery to be cancelled, also replace it? |
I didn't see #2407 before creating this. I think the main implementation difference is this PR keeps the event loop running in a separate thread to the blocking calls to stdin. The other PR will block the event loop each time the repl is waiting for input on stdin. At the moment, that probably doesn't make much difference, but I have my eye on being able to run tasks in the background. I've also got reasonable test coverage here. @CoolCat467 might have had other things in mind? |
|
Very similar approaches, both based on tweaking asyncio's solution. The other one uses a nursery it sends tasks to with |
Using eval only worked by accident, because "locals" was being passed into the "globals" value. InteractiveInterpreter in the stdlib correctly uses exec, but this doesn't work for us, because we need to get the values of the expression and check if it is a coroutine that need to be awaited. asyncio.__main__ uses the same type.FunctionType idea, which I originally avoided because I didn't really understand it and thought eval was simpler...
clint-lawrence
force-pushed
the
add-repl
branch
from
12127a5 to
bc57699
Compare
Even when it is in an exception group
|
I think all the previous feedback has been addressed. Please let me know if you need anything else for this to be merged. |
src/trio/_repl.py
Outdated
| func = types.FunctionType(code, self.locals) | ||
| try: | ||
| coro = func() | ||
| except BaseException as e: | ||
| return e | ||
|
|
||
| if inspect.iscoroutine(coro): | ||
| try: | ||
| await coro | ||
| except BaseException as e: | ||
| return e |
There was a problem hiding this comment.
Can some of this exception stuff be replaced with stuff from outcome? I don't see it providing much technical benefit but it would decrease some code duplication.
Looking at outcome's code, it looks like they remove one layer off the exception's traceback where this doesn't. That might be a good thing to do?
There was a problem hiding this comment.
I might not be understanding how outcomes works, but I don't see how it would simplify this. Wouldn't it just end up calling capture method and then immediately unwrapping?
try:
coro = outcome.capture(func)
coro.unwrap()
except BaseException as e:
return eThe traceback printing is consistent with the standard python repl and the python -m asyncio so, I'm leaning towards keeping it as is.
There was a problem hiding this comment.
Well specifically I was thinking directly returning the Value/Error (and an if on whether to use capture or acapture? Which would disallow leaving out the await when calling an async function but that's already trio style. I'll make this a separate comment when I get back to looking at this). If the trace backs are the same then the only benefit would be a few less lines, which probably isn't worth the work.
There was a problem hiding this comment.
I came up with this. It's better in some ways, but at the same time bit strange.
def runcode(self, code: types.CodeType) -> None:
async def _runcode_in_trio():
func = types.FunctionType(code, self.locals)
result = outcome.capture(func)
if isinstance(result, outcome.Error):
return result
coro = result.unwrap()
if inspect.iscoroutine(coro):
return await outcome.acapture(lambda: coro)
return outcome.Value(coro)
try:
value = trio.from_thread.run(_runcode_in_trio).unwrap()
except SystemExit:
# If it is SystemExit quit the repl. Otherwise, print the
# traceback.
# There could be a SystemExit inside a BaseExceptionGroup. If
# that happens, it probably isn't the user trying to quit the
# repl, but an error in the code. So we print the exception
# and stay in the repl.
raise
except BaseException:
self.showtraceback()I think it is awkward because there is a single step for most expressions, but any repl input that is an await needs the second step of evaluation.
There was a problem hiding this comment.
I finally tried poking at this a bit, ultimately what I was thinking about is:
def runcode(self, code: types.CodeType) -> None:
async def _runcode_in_trio():
func = types.FunctionType(code, self.locals)
if inspect.iscoroutinefunction(func):
return await outcome.acapture(func)
else:
return outcome.capture(func)
try:
value = trio.from_thread.run(_runcode_in_trio).unwrap()
except SystemExit:
# If it is SystemExit quit the repl. Otherwise, print the
# traceback.
# There could be a SystemExit inside a BaseExceptionGroup. If
# that happens, it probably isn't the user trying to quit the
# repl, but an error in the code. So we print the exception
# and stay in the repl.
raise
except BaseException:
self.showtraceback()probably doesn't work cause I haven't tested exactly that, but inspect.iscoroutinefunction seems to do the right thing (doesn't trigger for trio.sleep(1) or async def x(): ..., but triggers for await trio.sleep(1))
There was a problem hiding this comment.
Ah, iscoroutinefunction is the bit I was missing. That is much better. Thanks for the nudge in that direction. I've got a few bits to double check but it looks like that works right.
There was a problem hiding this comment.
Normally it might not be the best idea (since it can’t detect code that just returns a coroutine object directly). But in this case we know return isn’t valid.
|
This is probably here for good reason; in fact I get a feeling of deja-vu so we've probably already discussed this. If there's previous discussion about this, just link that and I should be satisfied. However, I don't really like the behavior of unconditionally I will eventually get around to poking at this and likely rediscover any issues with not unconditionally This would also fix the awkwardness around two-step evaluation! |
This isn't automatically awaiting the input. For example below the first |
|
Oh! That's what I get for assuming things from the code, rather than testing things. Sorry! |
src/trio/_repl.py
Outdated
| func = types.FunctionType(code, self.locals) | ||
| try: | ||
| coro = func() | ||
| except BaseException as e: | ||
| return e | ||
|
|
||
| if inspect.iscoroutine(coro): | ||
| try: | ||
| await coro | ||
| except BaseException as e: | ||
| return e |
There was a problem hiding this comment.
I finally tried poking at this a bit, ultimately what I was thinking about is:
def runcode(self, code: types.CodeType) -> None:
async def _runcode_in_trio():
func = types.FunctionType(code, self.locals)
if inspect.iscoroutinefunction(func):
return await outcome.acapture(func)
else:
return outcome.capture(func)
try:
value = trio.from_thread.run(_runcode_in_trio).unwrap()
except SystemExit:
# If it is SystemExit quit the repl. Otherwise, print the
# traceback.
# There could be a SystemExit inside a BaseExceptionGroup. If
# that happens, it probably isn't the user trying to quit the
# repl, but an error in the code. So we print the exception
# and stay in the repl.
raise
except BaseException:
self.showtraceback()probably doesn't work cause I haven't tested exactly that, but inspect.iscoroutinefunction seems to do the right thing (doesn't trigger for trio.sleep(1) or async def x(): ..., but triggers for await trio.sleep(1))
|
I've simplified the logic a little with the suggestion from @A5rocks and I think the should be good now. |
|
Cc @mikenerone @TeamSpen210 any comments before this gets merged? |
|
Hey @clint-lawrence, it looks like that was the first time we merged one of your PRs! Thanks so much! 🎉 🎂 If you want to keep contributing, we'd love to have you. So, I just sent you an invitation to join the python-trio organization on Github! If you accept, then here's what will happen:
If you want to read more, here's the relevant section in our contributing guide. Alternatively, you're free to decline or ignore the invitation. You'll still be able to contribute as much or as little as you like, and I won't hassle you about joining again. But if you ever change your mind, just let us know and we'll send another invitation. We'd love to have you, but more importantly we want you to do whatever's best for you. If you have any questions, well... I am just a humble Python script, so I probably can't help. But please do post a comment here, or in our chat, or on our forum, whatever's easiest, and someone will help you out! |
Closes #1360
This adds a repl that can be launched with
python -m trio, similar to asyncio and curio.I still need to add a news entry and update the documentation, but I was hoping for some feedback if this is likely to be accepted before doing that. Also, if there is any suggestions about where it would be best to document this that would be helpful.