Snapshot testing, type hints, refactoring

[lmmx]

I had a go at refactoring this and making the testing a bit more robust, including the examples from the README as tests and using inline-snapshot so that the exact same results are ensured. The refactored the run method reads more clearly now.

I initially split it out into separate modules to understand it but then put them back into one as they're all quite closely related :-) I put maincall on the Env since it's directly related, and used OOP references to self.temp_dir rather than repeatedly re-creating pathlib-type Path objects from it.

I also changed to use Pathlib.read_text()/read_bytes() as the code often had pathlib types to hand but used open.

I also figured out how to get the uv subprocess calls to run without shell=True, my understanding is it's preferable to use lists of command args rather than a string.

In the test suite I used pytest's importorskip to simply import standalone examples from the examples subfolder (which came from the examples in the README). This 'trick' of importing modules that aren't in the package requires the repo's root dir to be on the PATH, which PDM package manager does automatically. I noticed uv doesn't do this, so I added a 2 liner to conftest.py which will ensure all runs of the test suite do this.

There were some trivial bugs I caught:

• the argskwargs_to_callstring funcdef was not used, so I put it in an unused dir, it can probably be deleted (presume it was from early development)
• quiet = "--quiet" if not self.debug else "" was set but then in the if self.debug: the command was used with --quiet (ignoring the quiet variable and also the opposite logic) in Env.run(). I figured that if you're debugging you do want verbose output, i.e. the ternary variable was right.

Plus added some missing docstrings on maincall and completed the comments documenting Env.run()

I think that's about it, happy to explain anything else I forgot to mention :)

[koaning]

For future work it would be better to first open an issue to confirm the work that you're planning on doing. That makes it much easier for me to confirm if I agree with the direction and it might also be nice to be able to make smaller PRs. 21 files changes is a tad bit intimidating.

Will figure out some time to have a look.

[koaning]

It also seems like CI is not green.

I think there are some valid ideas in this PR, but would it be possible to make smaller PRs? Mainly to make the review work easier on my side?

[lmmx]

Yeah oh no prob at all, this was initially just to get my head around it and then got carried away :)

Edit CI was failing due to demo.py being refactored into individual examples, I put it back as was and expect CI should pass now, tests all passing on my end

[koaning]

To be clear, if you are willing to split up some of these PRs I will gladly accept them. But yeah ... 21 file changes are intimidating 😅

[lmmx]

Super, and makes a good opportunity to check my work :slightly_smiling_face: I'm on the case :saluting_face: I've kept a local copy but re-forked and attempting to make PRs that won't overlap. The PR branch names are prefixed with my username and a number indicating relative order if it helps you keep track (as well as the GitHub PR number):

• 11

• 12

• 13

• 14

• 15

• 16

As I can see these cover the changes I made to the package source module itself, the other changes were to the test suite

• 17

• 18

• 19