search

elcorto / psweep

Loop like a pro, make parameter studies fun.
https://elcorto.github.io/psweep
BSD 3-Clause "New" or "Revised" License
13 stars 2 forks source link

Please add __version__ to package main module #17

Closed akukuq closed 5 months ago

akukuq commented 6 months ago

psweep does not seem to follow the convention for storing package version in __version__ variable in the main package scope. This makes it difficult to distinguish between versions programmatically in user code.

My particular case where this became a problem was with the default database filename changing from 'results.pk' to 'database.pk'. My code used to rename that file from the expected default path to something else. After updating psweep to a newer version the default database location was different then previously. If psweep had __version__ set, I could have handled it with an if statement on the __version__ value.

On a related note, please also consider storing the default value of the database_basename argument as a constant, e.g. DEFAULT_DATABASE_BASENAME.

elcorto commented 6 months ago

Thank you for your interest in the project.

psweep does not seem to follow the convention for storing package version in __version__ variable in the main package scope. This makes it difficult to distinguish between versions programmatically in user code.

Good point, thanks. PRs for this are welcome! This should also get updated via bump2version, see .bumpversion.cfg.

My particular case where this became a problem was with the default database filename changing from 'results.pk' to 'database.pk'. My code used to rename that file from the expected default path to something else.

You can use run(..., database_dir="/path/to", database_basename="my_db.pickle") to change the database path and basename. Admittedly that's a bit cumbersome and one could easily add a simpler database_file="/path/to/my_db.pickle" extra argument with the downside of cluttering the API while maintaining backwards compatibility.

After updating psweep to a newer version the default database location was different then previously. If psweep had __version__ set, I could have handled it with an if statement on the __version__ value.

Can you specify at which version the database location (so database_dir) changed? database_dir (along with the change results.pk -> database.pk) was added in 077b7464c4c0046097c79c19fe004ae06bc7bbd7 to make the database directory configurable, but the default has always been calc_dir, which defaults to ./calc.

On a related note, please also consider storing the default value of the database_basename argument as a constant, e.g. DEFAULT_DATABASE_BASENAME.

With the exception of PSET_HASH_ALG, I'd only define (string) constants if they are used at least twice in the code. database_basename is assigned a default value once in the definition of run() and I don't feel that it deserves special treatment over other default values such as run(..., calc_dir="calc") or prep_batch(..., calc_templ_dir="templates/calc").