trio icon indicating copy to clipboard operation
trio copied to clipboard

tweaks to new REPL

Open richardsheridan opened this issue 1 year ago • 6 comments

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.

richardsheridan avatar May 18 '24 17:05 richardsheridan

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
@@           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 Coverage Δ
src/trio/_repl.py 100.00% <100.00%> (ø)
src/trio/_tests/test_repl.py 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

codecov[bot] avatar May 18 '24 17:05 codecov[bot]

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!

richardsheridan avatar May 18 '24 17:05 richardsheridan

Cc @clint-lawrence cause you made the original PR

A5rocks avatar May 18 '24 18:05 A5rocks

Out of date branch so updated

CoolCat467 avatar May 18 '24 21:05 CoolCat467

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

richardsheridan avatar May 18 '24 23:05 richardsheridan

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 avatar May 18 '24 23:05 richardsheridan

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 avatar May 25 '24 16:05 richardsheridan

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.

richardsheridan avatar May 26 '24 20:05 richardsheridan