search

HiSPARC / sapphire

SAPPHiRE, a framework for HiSPARC
https://docs.hisparc.nl/sapphire/
GNU General Public License v3.0
7 stars 9 forks source link

Modernize with new tooling and fix or update some code #205

Closed 153957 closed 5 months ago

153957 commented 6 months ago

Apply ruff fixes and formatting. Configure ruff instead of flake8. Add editorconfig Replace setup.py by pyproject.toml Fix/ignore remaining ruff errors Fix broken tests. Support Python 3.12 Drop support for Python 3.8 Upgrade some old script to support Python 3 Update required version for dependencies, compatible with Python 3.9-3.12.

davidfokkema commented 5 months ago

Nice, looking at it now. Just curious: what Python features are we now using that required dropping 3.8? Just to be clear: I'm all for dropping support for such an old version. Also the first time I've heard of .editorconfig. Nice!

153957 commented 5 months ago

Just curious: what Python features are we now using that required dropping 3.8?

This is because Numpy 1.25 requires 3.9-3.11 (and numpy 1.26 for 3.12) And this version of Numpy also had other changes relating to random number generators and dtype promotion, i.e. when an (u)int64 is converted to float64 or not. It was hard to also keep all our tests compatible with earlier versions of numpy.

153957 commented 5 months ago

Regarding the use of default_rng(), is it also possible to use it on a per module basis? Import numpy, define rng = np.random.default_rng(), and use rng.uniform() throughout the module?

It might be, hmm, that might actually be a good idea. I have a local stash where I tried to create new default_rng during the init of certain classes (simulations) where we also set a seed during the tests to ensure stable results. However, a lot of those classes currently use classmethods, which would have to turn into regular methods to be abled to use the class bound instance of the RNG, but that makes the tests far more complicated since some things in the classes needs to be mocked.. Creating it at module level might be easier. But since numpy still allows the use of its global RNG I wont work on this now.

davidfokkema commented 5 months ago

However, a lot of those classes currently use classmethods, which would have to turn into regular methods to be abled to use the class bound instance of the RNG, but that makes the tests far more complicated since some things in the classes needs to be mocked..

Ah yes, that makes it hard to use a class instance.

Creating it at module level might be easier. But since numpy still allows the use of its global RNG I wont work on this now.

Fair! We don't require the improved algorithms.