Skip to content
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

core.constants.py #129

Closed
vgro opened this issue Dec 7, 2022 · 4 comments
Closed

core.constants.py #129

vgro opened this issue Dec 7, 2022 · 4 comments

Comments

@vgro
Copy link
Collaborator

vgro commented Dec 7, 2022

Different modules will need access to a core set of constants such as density of air, Stephan Bolzman constant,...

Proposed solution: centrally stored dictionary core.constants.py

@vgro vgro changed the title core.constants. py core.constants.py Dec 7, 2022
@davidorme
Copy link
Collaborator

davidorme commented Dec 7, 2022

I think that makes sense as a mini module. Another possibility for the contents - which I used over in pyrealm - is to use dataclasses.dataclass. There are a couple of possible advantages:

  • You can freeze the contents, so no-one can accidentally edit them.
  • It gives you a slightly neater access method (constants.density_of_air not constants['density_of_air'])

@dalonsoa
Copy link
Collaborator

dalonsoa commented Dec 7, 2022

The second point is easily covered by by putting all the constants in a module and using it as from vr.core import constants. Granted that it does not prevent importing specific constants nor to edit them, but it seems like the most straight forward approach.

Either way (data class or module) will work fine. My main concern always with universal constants are units. What units they will be defined in? How is that communicated to the users? I tried a not-so-popular approach in a project some time ago using pint, but yet it might work here.

@davidorme
Copy link
Collaborator

Completely agree about care needed on units. There are various python solutions for units such as pint and cfunit but it does add a layer of complexity - I've also seen pushback against using coded units. We certainly should state them in docs/comments and perhaps we park whether to actually programatically set them for now?

@davidorme
Copy link
Collaborator

Closing - see #364

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

No branches or pull requests

3 participants