-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A generic units class for pyiron #397
Conversation
(Jörg's suggestion)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few small remarks, but generally looks really nice!
I especially liked the detailed usage examples, do they pass under doctest? No problem if they don't, tough, but it's easy to check and a nice bonus.
code_units.add_quantity(quantity="energy", | ||
unit=pint_registry.kilocal / (pint_registry.mol * pint_registry.N_A)) | ||
code_units.add_labels(labels=["energy_tot", "energy_pot"], quantity="energy") | ||
# Raise Error for undefined quantity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments like this could live inside self.assertRaises(..., msg='Raise Error for undefined quantity')
to make the test output a little bit more readable.
Co-authored-by: Marvin Poul <[email protected]>
Co-authored-by: Marvin Poul <[email protected]>
Co-authored-by: Marvin Poul <[email protected]>
Co-authored-by: Marvin Poul <[email protected]>
Co-authored-by: Marvin Poul <[email protected]>
� Conflicts: � pyiron_base/generic/units.py
I've found a way to test the Docstrings properly as well. If this is also okay, I can merge this! |
Cool! I had meant to just check it once, but this will make keeping the docstring up to date much easier, thanks! |
Leveraging the capabilities of pint to handle units for different simulation codes as well as modelling length and timescales.