python-trio / trio

Trio – a friendly Python library for async concurrency and I/O
https://trio.readthedocs.io
Other
5.98k stars 325 forks source link

tweaks to new REPL #3002

Closed richardsheridan closed 1 month ago

richardsheridan commented 1 month ago

A follow up to #2972. These would have been review comments earlier if I knew the code was going to be (a) this simple and (b) in my wheelhouse!

I think runcode can just be:


    def runcode(self, code: types.CodeType) -> None:
        try:
            func = types.FunctionType(code, self.locals)
            if inspect.iscoroutinefunction(func):
                trio.from_thread.run(func)
            else:
                trio.from_thread.run_sync(func)
        except SystemExit:
            # ...snip...
            raise
        except BaseException:
            self.showtraceback()

It's less code, less layers of onion unwrapping, and the more common from_thread.run_sync is more efficient (not that anyone would notice latency from two extra checkpoints at the REPL). That's the first commit.

I also noticed one fragility, which is that KI works thanks to the thread task reentry feature combined with the fact that we inject KeyboardInterrupt into the main task. If we were to go the route of https://github.com/python-trio/trio/issues/733#issuecomment-629126671 and just cancel everything and transmute to KeyboardInterrupt when the run finishes, the run would be cancelled. I put a test for this in the second commit.

Finally, we can hide the threading code frames from the traceback, although I'm not sure about editing the global exception state like that. that's the last commit.

codecov[bot] commented 1 month ago

Codecov Report

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

Project coverage is 99.63%. Comparing base (3350c11) to head (b4af483).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #3002 +/- ## ======================================= Coverage 99.63% 99.63% ======================================= Files 120 120 Lines 17783 17865 +82 Branches 3197 3212 +15 ======================================= + Hits 17718 17800 +82 Misses 46 46 Partials 19 19 ``` | [Files](https://app.codecov.io/gh/python-trio/trio/pull/3002?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/\_repl.py](https://app.codecov.io/gh/python-trio/trio/pull/3002?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/3002?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%> (ø)` | | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/python-trio/trio/pull/3002/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-trio)
richardsheridan commented 1 month ago

Looks like that traceback cleanup trick only works on 3.11 later. Either we could find a different trick or only assert it on recent python versions!

A5rocks commented 1 month ago

Cc @clint-lawrence cause you made the original PR

CoolCat467 commented 1 month ago

Out of date branch so updated

richardsheridan commented 1 month ago

The difference in editing exceptions that showed up in 3.11 is documented so I think we can just rely on it.

richardsheridan commented 1 month ago

And to follow up on people interested in a nursery for background tasks, my test accidentally shows that you can just use the system nursery.

https://github.com/python-trio/trio/blob/8a0b573c57442b4ea5a28ae154d31b0ee8b0a08a/src/trio/_tests/test_repl.py#L106-L117

richardsheridan commented 1 month ago

A really clean (IMO) implementation breaks certain features of InteractiveConsole intended to enable customization in subclasses. So I made it final to discourage people from trying and noted in comments which things won't work for future contributors to this class.

richardsheridan commented 1 month ago

Not immediately obvious that raising the error in that branch causes error to be only printed and not close repl if I am reading the comment right.

Yes, I think I did a poor job of splitting up that comment. I'm going clarify that a bit and then someone can do a nice squash merge for us.