At present the major Euphonic classes have some magical treatment of array Quantity attributes:
__init__ includes a lot of validation logic and sets up the storage model:
- they are stored as separate array and unit attributes.
- Public getters produce Quantity on-the-fly, while
- private access to values in atomic units might be intended for use in optimised C extensions.
- Generally arrays cannot be modified by working on slices, but this fails silently.
- Modifications are made by working a copy of the attribute and then setting it back to the parent object.
I propose that instead:
- Classes inherit
@dataclass and store Quantity directly,
- as immutably as possible (by using dataclass
frozen=True and setting the writeable flags of numpy arrays to False).
- Validation logic can be retained using the dataclass
__post_init__ hook (which would also be responsible for setting numpy flags).
- If atomic-units versions are required for C extension calls, this conversion can be done in the functions that wrap them. If this is likely to happen repeatedly, it could be delegated to a
@cachedproperty on the dataclass.
- Modifications should be done by building a new class; this can be streamlined by providing a
__replace__ method, which from Python 3.13 is accessed by copy.replace().
Discussed and tested in #483 #484 for Crystal; this design seems very viable. But as a sweeping API change that requires significant updates to examples in docs, let's push it back to v3. (In fact, this could be the main component of v3 and we don't have to wait too long to do it.)
At present the major Euphonic classes have some magical treatment of array Quantity attributes:
__init__includes a lot of validation logic and sets up the storage model:I propose that instead:
@dataclassand store Quantity directly,frozen=Trueand setting thewriteableflags of numpy arrays to False).__post_init__hook (which would also be responsible for setting numpy flags).@cachedpropertyon the dataclass.__replace__method, which from Python 3.13 is accessed bycopy.replace().Discussed and tested in #483 #484 for Crystal; this design seems very viable. But as a sweeping API change that requires significant updates to examples in docs, let's push it back to v3. (In fact, this could be the main component of v3 and we don't have to wait too long to do it.)