Template for modules

[JanCBrammer]

@robertoostenveld
I have a question about the design of the EEGsynth modules. I like the recent re-organization of how the modules are constructed with generic functions (setup, start, loop_once etc.) that are consistent across all (most?) of the modules. I like it because it makes the code much clearer. I was wondering if it would be an option to have an even more generic template for modules. I was thinking of something like this:

class Module():

    def __init__(self):
        self.patch = foo
        self.redis = bar
        self.more_attributes = baz
        
    def setup(self):
        "do stuff"
        
    def start(self):
        "do stuff"
        
    def loop_once(self):
        "do stuff"
        
    etc.

When someone wants to implement a new module they can sub-class the module template and modify or extend it the way they like. In the simplest case only the loop_once method would be extended.

class HeartRate(Module):

    def __init__(self):
        "add heart-rate specific attributes if necessary"
    
    def loop_once(self):
        "modify and/or extend with peak detection or something"

Would this be a reasonable pattern or am I overlooking something (which is more likely than not I guess)? I think it could make the code even simpler to read since it further reduces boiler-plate and avoids global variables.

[robertoostenveld]

Hi Jan,

I have been thinking about this as well (early on, prior to starting the actual refactoring) and back then decided against it. I mainly have to deal with Stephen in working together on the code, and his Python skills are less than mine (and my own are not so great either). When discussing this with Stephen, it was clear that he still did not get this whole OO business with classes, objects etc.

The reason not to go directly in this direction was that it might be too complex, i.e. overengineered and possibly hamper contributions of people that just manage a flat structured python script. Also, it might have made the refactoring work simply too much to handle.

However, now that the code has all been reorganized and cleaned up during the process, it seems like a much more manageable task to impose additional structure in the form you propose. It would further reduce duplication, prevent errors, etc.

AFAIK there are three modules for which it would not work: Redis, buffer, and openbci2ft. I am also not sure whether we should continue supporting them in the current form since they don't work any way when one does pip install eegsynth.

Perhaps to probe you a bit further: in the refactoring, I decided (for the time being) to make all variables in a module global to not disrupt the previously working code. I even added some code to each module to identify non-global variables (e.g. here). Would you now a better way of handling with this that would be manageable to implement robustly in all ~50 modules (i.e. ~16000 lines of code).

[JanCBrammer]

It makes sense that immediately jumping to OOP during the refactoring was one step too far. I think that OOP presents itself as a salient solution only now that the structure of a module is more obvious.

Don't get me wrong, I don't think that globals are bad per se, I only find it harder to get an overview of what is happening when I cannot explicitly see what arguments a function takes and what it returns (or what state it introduces). Especially since there are some globals that are only relevant in
one function. For example response, parser, args, config never "leave"

eegsynth/module/clockmultiplier/clockmultiplier.py

Line 113 in 2bc8a80

def _setup():

.

I wouldn't know how to deal with the globals other than replacing them with explicit arguments in __main__:

if __name__ == '__main__':
    patch, redis_cli = _setup()    # _setup() returns the objects required by subsequent functions
    thread, monitor = _start(patch, redis_cli)
    try:
        _loop_forever(thread, monitor, patch, redis_cli)
    except (SystemExit, KeyboardInterrupt, RuntimeError):
        _stop()

This would be more explicit to read. But going through all modules and hand-picking the variables that need to be passed around in __main__ is probably not feasible if all that is gained is readability.