Add a trio repl - Githubissues

clint-lawrence commented 6 months ago

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.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.64%. Comparing base (564907b) to head (d6f4bd1).

Additional details and impacted files

```diff @@ 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 ``` | [Files](https://app.codecov.io/gh/python-trio/trio/pull/2972?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio) | Coverage Δ | | |---|---|---| | [src/trio/\_\_main\_\_.py](https://app.codecov.io/gh/python-trio/trio/pull/2972?src=pr&el=tree&filepath=src%2Ftrio%2F__main__.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX19tYWluX18ucHk=) | `100.00% <100.00%> (ø)` | | | [src/trio/\_repl.py](https://app.codecov.io/gh/python-trio/trio/pull/2972?src=pr&el=tree&filepath=src%2Ftrio%2F_repl.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3JlcGwucHk=) | `100.00% <100.00%> (ø)` | | | [src/trio/\_tests/test\_repl.py](https://app.codecov.io/gh/python-trio/trio/pull/2972?src=pr&el=tree&filepath=src%2Ftrio%2F_tests%2Ftest_repl.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio#diff-c3JjL3RyaW8vX3Rlc3RzL3Rlc3RfcmVwbC5weQ==) | `100.00% <100.00%> (ø)` | |

oremanj commented 6 months ago

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.

A5rocks commented 6 months ago

Also would it be worth it to have a nursery created by default in a nursery local so you can start something in the background for the REPL?

(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.

CoolCat467 commented 6 months ago

I like this pull request a lot more than #2407

clint-lawrence commented 6 months ago

I am in favor of accepting something like this. As the github bot has noted, please add type hints.

Yes, I have start on that. Just a few issues I need to come up with a fix for.

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.

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!

Also would it be worth it to have a nursery created by default in a nursery local so you can start something in the background for the REPL?

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

jakkdl commented 6 months ago

I like this pull request a lot more than #2407

to save me+others some time comparing them if you already have - why? What's the difference between their approach?

TeamSpen210 commented 6 months ago

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?

clint-lawrence commented 6 months ago

I like this pull request a lot more than #2407

to save me+others some time comparing them if you already have - why? What's the difference between their approach?

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?

CoolCat467 commented 6 months ago

Very similar approaches, both based on tweaking asyncio's solution. The other one uses a nursery it sends tasks to with nursery.start_soon, adds change log message, and adds docs reference to the fact that this feature exists now, but is missing tests and I believe it doesn't work correctly because among other things, main nursery never runs because console.interact is blocking trio from ever running. Prior to me stepping in a few months ago after finding it when looking through currently open pull requests, it had been stale for quite some time and didn't have type annotations. I like this new proposed approach a lot more and suggest stealing the documentation and change log additions the other one made.

clint-lawrence commented 6 months ago

I think all the previous feedback has been addressed. Please let me know if you need anything else for this to be merged.

A5rocks commented 5 months ago

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 awaiting the result of the thing in the prompt. Is this really necessary? Or can we use some inspect function to find that there's an await?

I will eventually get around to poking at this and likely rediscover any issues with not unconditionally awaiting the result, but if there's no issue then I personally would really prefer something where directly calling trio.sleep(5) doesn't automatically await unless you add an await yourself.

This would also fix the awkwardness around two-step evaluation!

clint-lawrence commented 5 months ago

However, I don't really like the behavior of unconditionally awaiting the result of the thing in the prompt. Is this really necessary? Or can we use some inspect function to find that there's an await?

This isn't automatically awaiting the input. For example below the first trio.sleep(1) does not get awaited or actually sleep. The second await trio.sleep(1) blocks for 1 second, before the repl prompts for the next input.

trio REPL 3.12.2 (v3.12.2:6abddd9f6a, Feb  6 2024, 17:02:06) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Use "await" directly instead of "trio.run()".
Type "help", "copyright", "credits" or "license" for more information.
>>> import trio
>>> trio.sleep(1)
<coroutine object sleep at 0x10f3eda40>
>>> await trio.sleep(1)
>>>

A5rocks commented 5 months ago

Oh! That's what I get for assuming things from the code, rather than testing things. Sorry!

clint-lawrence commented 5 months ago

I've simplified the logic a little with the suggestion from @A5rocks and I think the should be good now.

A5rocks commented 5 months ago

Cc @mikenerone @TeamSpen210 any comments before this gets merged?

trio-bot[bot] commented 4 months ago

Hey @clint-lawrence, it looks like that was the first time we merged one of your PRs! Thanks so much! :tada: :birthday:

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:

Github will automatically subscribe you to notifications on all our repositories. (But you can unsubscribe again if you don't want the spam.)
You'll be able to help us manage issues (add labels, close them, etc.)
You'll be able to review and merge other people's pull requests
You'll get a [member] badge next to your name when participating in the Trio repos, and you'll have the option of adding your name to our member's page and putting our icon on your Github profile (details)

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!

python-trio / trio

Add a trio repl #2972

Codecov Report