change saqc functions to SaQC.methods
Although this was more an by-product of the bolt-on of the Programming Interface, I think the clear separation of test function and the core
class has served as well until now. But the time might have come, to actually rethink the whole idea/solution. The main driving force behind the issue at hand are several non-ideal implementation quirks, that accumulated over time, the most severe IMO are the following:
- We duplicate and refactor our function signatures at two distinct occasion, 1. during the doc generation in an automated fashion and 2. for in
saqc.core.modules
. Both bring their own workarounds ($106) and bugs (e.g. broken doc module names). - The only way to get information into and out of the test functions is through their parameter-list and return-tuple. In general, a really nice way to implement functions, I guess it became an hindrance over time. We implement the not (exclusively) local behavior of certain function (like harmo-deharmo combinations) through faulty workarounds (#175 (closed)) and the large amount of test functions (and all the signature duplication) makes it quite hard to get even simple changes to the interface in (like the resorting from
data, field, flagger
->data, flagger, field
).
That's why I'd like to make the following proposal. Let's make 'real' methods out of out test functions with the follong interface:
flagFoo(self: SaQC, field: str, *args, **kwargs): -> SaQC
To achieve this, we could, for example, move the actual implementations of our functions into the saqc.core.module
classes (but of course, there are many different ways to get this done). As always, there are pros and cons to this.
The Cons (I see):
- The most obvious con is, that it would be quite some work: the fact, that we wouldn't pass
data
andflagger
explicitly anymore, we need to refactor all test functions to access the respectiveSaQC
-attributes (mostly annoying but dump refactoring). But this would also imply changes to the not-yet-merged rework of the flagger in !231 (merged), where we might need (?) some more conceptional changes (looking at youcore.register.CallSate
. - We tightly couple the functions to the
core
-Object and therefore loose the possibility to call test functions independently.
The Pros (I see):
- Our implemented interface and doc strings would equal to our Programming Interface.
- We could (ab)use
self: SaQC
to store intermediate products. - Function could access the function call history and its future (through
saqc._to_call
) to for example check if a harmo is followed by a deharmo or not
So we could immediately close #180 (closed) and there would be straight forward solution to #175 (closed) (namely an dedicated SaQC
attribute only holding intermediate variables).
To summarize, I like the clear separation of the core and test functions, and from my conceptional point of view, our current implementation is superior than a method-based approach could ever hope to be. BUT (yes, this is an all uppercase but) from a practical standpoint (as in our main target is Python 3.8) it led to workarounds I consider even less appealing...