Skip to content

Major package rewrite: structure, formatting, testing etc...#23

Open
martinjohndyer wants to merge 40 commits intomainfrom
linting
Open

Major package rewrite: structure, formatting, testing etc...#23
martinjohndyer wants to merge 40 commits intomainfrom
linting

Conversation

@martinjohndyer
Copy link
Collaborator

This is the big package rewrite and tidy up.

Some highlights are:

  • Reworking the structure, for instance moving most functions into larger modules, having separate data and script submodules.
    • There are now only a handful of more general modules:
      • snf_simulations.data: handles loading data files and spectra
      • snf_simulations.spec: functions for creating spectra
      • snf_simulations.cask: contains the function for calculating the total spectrum of an SNF cask
      • snf_simulations.physics: various functions to do with nuclear physics calculations
    • The command_line.py script is now package entry point snf-sim, which should be available on the command line when the package is installed.
    • Most hardcoded data, like isotope fractions for the different reactors and atomic properties, are now stored in CSV files. Ultimately these should be replaced by a mix of input files (Add input configuration options #17), automated requests (Verify antineutrino data files #16) and the mendeleev package (Clarify atomic/molar masses and find better source #21).
  • Replacing setup.py with a new pyproject.toml including package metadata (see Improve package metadata #8).
  • Full code formatting and linting with ruff, including a GitHub Action workflow to check future pull requests (to run locally use ruff format & ruff check).
  • Added full docstrings to each function, including type hints (Improve internal documentation #10), paving the way for full documentation (Create external documentation #11)
  • (Almost) complete testing suite (Add testing and CI #6).
    • I say almost, because the code coverage is actually down from Add initial testing functions #18 at a somewhat depressing 59%. That's actually very heavily weighted towards the command_line.py script, which can't be tested since its main purpose is to produce ROOT windows. Coverage of the rest of the package is 95%+, with the exceptions being somewhat awkward cases like data files being unavailable (which again should be reworked in the future).
    • Automated testing is also not currently enabled, I tried to make a GitHub Action to run Pytest but again installing ROOT is a major hurdle.

There were also a handful of new bugs I came across while putting this together, which are all now fixed:

Some things I didn't get around to doing:

  • I think it would be more usable to make the module more classed base, e.g. with a Spectrum class for the basic unit and then a Cask class which could combine spectra (you can see the idea in how the modules are named). I did start working on this, but a lot of it would be built around the ROOT classes, especially ROOT.TH1D and ROOT.TList. So rather than doing a load of work now that would end up being removed, I'll wait until Look at removing the dependency on ROOT #14 which I think deserves its own PR (and is probably the next task anyway).
  • Another thought was to replace the command_line.py script with something like a Jupyter Notebook, since it's really more of a demo of the various features of the package. But again that doesn't really work with ROOT plots. So the snf-sim script works for now, and when ROOT is replaced that can be reworked into a more general command line interface (which might also include launching the actual GUI too).
  • I also didn't touch the README yet, since it will need rewriting along with the full documentation and that should wait until the API is finalised.

@martinjohndyer martinjohndyer added this to the RSE project milestone Feb 9, 2026
@martinjohndyer martinjohndyer self-assigned this Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments