JYU-IBA / potku

Potku is analysis and simulation software for ToF-ERD measurements
https://www.jyu.fi/science/en/physics/research/infrastructures/accelerator-laboratory/pelletron/potku/
GNU General Public License v2.0
7 stars 7 forks source link

Convert the default reference density and the profile reference density to ENUMs #220

Closed jkpnen closed 3 years ago

jkpnen commented 3 years ago

Fix #219

tpitkanen commented 3 years ago

The idea here is right (constant values shouldn't be defined as "magical numbers" in many places), but the solution isn't.

Enums are used for comparing by identity (x is MyEnum.A), while normal constants are used for their values.

Was this change tested by the way? Enum members don't work like this:

>>> from enum import Enum
>>> class MyEnum(Enum):  # Oops, this does work if it inherits float, like it does in this pr
...  A = 10.0
...  B = 12.0
...
>>> MyEnum.A * 2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for *: 'MyEnum' and 'int'

Fix: define the values as normal constants in modules/profile.py instead, and import them from there to the other files.

# modules/profile.py

REFERENCE_DENSITY = 4.98e22
PROFILE_REFERENCE_DENSITY = 3.0  # TODO: Shouldn't this have e22?
# modules/get_espe.py

from .profile import PROFILE_REFERENCE_DENSITY, REFERENCE_DENSITY
tpitkanen commented 3 years ago

Update: my bad, this does work because the DefaultReferenceDensity inherits float (which I didn't notice the first time around). Sorry about questioning the testing.

Still, there's no need to use enums when plain constants work. Enums are for identity (e.g. Club, Diamond, Heart, and Spade for cards), not arithmetic (e.g. 123.45 * 67 + 89).