The use of dataclasses in `IW2D/interface.py` needs a closer look
The dataclasses
module is mainly meant to remove boilerplate code for simple classes. While this does not exclude it from being useful for more complex classes, it requires a more careful implementation. For clarity, IW2DLayer
, FlatIW2DInput
and RoundIW2DInput
all currently (MR !11 (merged) ) implement all of the following special methods:
__init__
__repr__
-
__eq__
,__ne__
__hash__
Notably, it does not implement:
-
__gt__
,__ge__
,__lt__
,__le__
Some issues or concerns about the current (MR !11 (merged) ) implementation of
- Since
IW2DLayer
is designed to be interchangable with any abstract Layer class, and no checks are done to confirm these are also immutable, the__hash__
method might be unsafe. This may cause broken behaviour if the objects are added to sets or used as keys in dictionaries. - Hashing of the input objects is also used by the pywit package to cache the results of IW2D calculations. This requires that objects produce the same results, but are not the same (e.g if they use different callables that produce the same results) still give the correct hash. This is already somewhat taken care of by implementing a method specifically for this task, but it should be discussed more if this approach is correct.
- Similarly, there should be a conscious decision whether
==
and!=
use actual equality (as it does currently) or equality of outputs. - For development it is very useful to have a complete
repr
. Now the layer sequences in the input objects are removed from theirrepr
's so that therepr
can be used to make reproducible hashes. This should be reverted, and the reproducible hash function should be written without the use of the repr.
- It could maybe use a
__str__
method, though
- It is an annoying quirk of dataclasses that inheritance is a bit broken by default arguments. I would be very nice to have a (perhaps abstract) parent class of
FlatIW2DInput
andRoundIW2DInput
, but this is unfeasable without giving all arguments defaults or dropping dataclasses (or at leas__init__
)entirely - Once the interface is done some more strict type verification should be done, either in the classes or in functions generating them.
My suggestion is that once the interface is "complete", these issues are discussed in a short meeting with everyone involved in IW2D and pywit development, and that solutions are implemented based on those discussions.