2

I am currently designing a configuration screen for my application. There are about 50 configuration parameters, which I have described in a CSV file, and read in Python.

Currently the fields for each parameter row are:

  • csv_row_count
  • cat_name
  • attr_name
  • disp_str
  • unit
  • data_type
  • min_val
  • max_val
  • def_val
  • incr
  • cur_val
  • desc

So a few examples for a parameter could be: Image showing 3 parameter rows in CSV file

Every single parameter is then drawn in my application as UI for the user to interact with, and looks like this: GUI for user to interact with, filled with information from CSV

The csv_row_count is later used as an idx for looping. If I ever want to add another parameter to be configured, I would have to use a unique one and add the parameter to the bottom of the CSV.

I currently have a class which stores all the information into attributes and can be stored back to the CSV after changes by the user.

Now, the problem I am having is that I need to react to the changes the user makes in the configuration screen (uniquely for every single parameter).

So my idea was to create an ConfigEventHandler class, that has an Event for when the parameter is created (on boot of the application, to draw it to the UI for example) and an Event which is fired when the value has been changed (to make changes to the value in the UI, and also react correspondingly for this specific setting that has been changed).

That class looks like this:

from axel import Event
import logging as log

class ConfigEventHandler:
    def __init__(self):
        # Make an Event for when a config row is created (usually only at boot of application)
        self.on_create_ev = Event()
        self.on_create_ev += self._on_create_event

        # Make an Event for when a config row has changed. Fire-ing of this event will be after a change in the attribute.
        self.on_change_ev = Event()
        self.on_change_ev += self._on_value_change_event

    def _on_create_event(self, idx):
        pass
        # log.info("Event OnCreate fired for IDX " + str(idx))

    def _on_value_change_event(self, idx, user_dict):
        log.info("Event On Value Change fired for IDX " + str(idx))
        log.info("With user dict: " + str(user_dict))

And I fire these events on creation of the class and when the values change. The only way for this events to identify which value has been changed, is by the idx value which correlates to the csv_row_count attribute as described in the CSV.

This would mean that if I want to react to each parameter being changed, I would need to fill the _on_value_change_event method with a lot of if-statements, e.g.:

if idx == 0:
    react_to_param0()
elif idx == 1:
    react_to_param1()
etc...

I feel like this is really bad practice (in any program language), and would easily break if someone screwed around with the idx numbering in the CSV file (E.G. me in a few months, or the person that will take over my project).

So my question is, what would be a good alternative for fixing this relationship issue in a way that is more clean than a lot of if-statements and a relationship between a list idx and CSV index to make it more future proof, and keep the code readable?

I have thought of making a seperate class instance for each congifurable parameter, but don't really see how that would break the 'fragile' seeming relation between the csv_row_count/idx and the unique functions that react to changes to one specific parameter.

martineau
  • 119,623
  • 25
  • 170
  • 301
Mats de Waard
  • 139
  • 2
  • 15
  • 1
    So you're going to write a `react_to_paramX()` function for every parameter? Does each one really need to be handled differently? What do you envision these functions doing exactly? I think this part of your design should be addressed first. In other words I strong think this might be an [XY Problem](https://xyproblem.info/). – martineau Sep 29 '21 at 16:13
  • @martineau Thanks for your reply. I might have phrased my question incorrectly. Your analogy to the XY problem sounds correct hahaha. Indeed, like you stated, each parameter has a unique reaction, which means I need to call a seperate function for each parameter somewhere in my code. My question about this is, how can I realize such a solution, in a way that stays future proof, readable and editable later on in my project (or when someone else takes over the project). I feel like adding a dictionairy (or list) that corresponds to a function name by IDX, does not comply with these requirements – Mats de Waard Sep 30 '21 at 07:59
  • Please explain in more detail what this `idx` value is and how it relates to the CSV. I have several ideas on how to improve things but want to make sure I understand what you're trying to do better first, before possibly posting an answer. – martineau Sep 30 '21 at 09:18
  • I edited the original question to be more descriptive and add images, I hope this makes my problem a bit more clear, and less XY :) Thanks for taking the time to help me with this design problem. – Mats de Waard Sep 30 '21 at 10:31
  • The information you added isn't really enough for me to help with the internal design of your app—it's mostly about how it operated and looks visually. However the details about the `idx` value and CSV file fields *do* allow me to suggest a good way to read (and write if you wished) that information which I have posted in an answer below. Since I can't really help with overall design, the second half of that answer describes a way to avoid all the repetitive `if`/`elif` coding in the `ConfigEventHandler` class' `_on_value_change_event()` method. – martineau Oct 01 '21 at 16:33

3 Answers3

1

A possible solution would be having a dictionary attribute that stores the function names like this:

def __init__(self):
    self.params_reactions = {
        0: self.react_to_param1,  # Each value is a reference of a function in the class
        1: self.react_to_param2,
        2: self.react_to_param3
        # Continues...
    }

And then you could just call the reaction function if the index is present and matches a key of the dictionary:

def _on_value_change_event(self, idx, user_dict):
    # Your code as well

    if idx in self.params_reactions:
        self.params_reactions[idx].__call__()

    # You could raise an exception if 'idx' is not found
    # It depends on how you want to treat a possible exception to the program's flow
martineau
  • 119,623
  • 25
  • 170
  • 301
NickS1
  • 496
  • 1
  • 6
  • 19
  • 1
    Rather than explicitly calling the function's `__call__()` method, you could simply call the the function: i.e. `self.params_reactions[idx]()`. It would also be more "pythonic" to ***not*** check `if idx in self.params_reactions:` and instead wrap the function call in a `try`/`except KeyError`. – martineau Oct 04 '21 at 15:07
1

Doing things in a "pythonic" way often involves using dictionaries, one of the language's primary data-structures which have been highly optimized for speed and minimal memory usage.

With that in mind, the first thing I'd do is to get the configuration parameters in the CSV file into one. Since there's one per row of the CSV file, a logical thing to do would be to use a csv.DictReader to read them since it returns each row as a dictionary of values. You can easily build a dictionary-of-dictionaries out of those by using the unique csv_row_count (aka idx) field of each one as the key for upper-level dict.

Here's what I mean:

import csv
from pprint import pprint

CONFIG_FILEPATH = 'parameters.csv'

config_params = {}
with open(CONFIG_FILEPATH, 'r', newline='') as configfile:
    reader = csv.DictReader(configfile, escapechar='\\')
    csv_row_count = reader.fieldnames[0]  # First field is identifier.
    for row in reader:
        idx = int(row.pop(csv_row_count))  # Remove and convert to integer.
        config_params[idx] = row

print('config_params =')
pprint(config_params, sort_dicts=False)

So if your CSV file contained the rows like this:

csv_row_count,cat_name,attr_name,disp_str,unit,data_type,min_val,max_val,def_val,incr,cur_val,desc
15,LEDS,led_gen_use_als,Automatic LED Brightness,-,bool,0.0,1.0,TRUE,1.0,TRUE,Do you want activate automatic brightness control for all the LED's?
16,LEDS,led_gen_als_led_modifier,LED Brightness Modifier,-,float,0.1,1.0,1,0.05,1,The modifier for LED brightness. Used as static LED brightness value when 'led_gen_use_als' == false.
17,LEDS,led_gen_launch_show,Enable launch control LEDs,-,bool,0.0,1.0,TRUE,1.0,TRUE,Show when launch control has been activated\, leds will blink when at launch RPM

Reading it with the above code would result in the following dictionary-of-dictionaries being built:

config_params =
{15: {'cat_name': 'LEDS',
      'attr_name': 'led_gen_use_als',
      'disp_str': 'Automatic LED Brightness',
      'unit': '-',
      'data_type': 'bool',
      'min_val': '0.0',
      'max_val': '1.0',
      'def_val': 'TRUE',
      'incr': '1.0',
      'cur_val': 'TRUE',
      'desc': 'Do you want activate automatic brightness control for all the '
              "LED's?"},
 16: {'cat_name': 'LEDS',
      'attr_name': 'led_gen_als_led_modifier',
      'disp_str': 'LED Brightness Modifier',
      'unit': '-',
      'data_type': 'float',
      'min_val': '0.1',
      'max_val': '1.0',
      'def_val': '1',
      'incr': '0.05',
      'cur_val': '1',
      'desc': 'The modifier for LED brightness. Used as static LED brightness '
              "value when 'led_gen_use_als' == false."},
 17: {'cat_name': 'LEDS',
      'attr_name': 'led_gen_launch_show',
      'disp_str': 'Enable launch control LEDs',
      'unit': '-',
      'data_type': 'bool',
      'min_val': '0.0',
      'max_val': '1.0',
      'def_val': 'TRUE',
      'incr': '1.0',
      'cur_val': 'TRUE',
      'desc': 'Show when launch control has been activated, leds will blink '
              'when at launch RPM'}}

The Dispatching of the Functions

Now, as far as reacting to each parameter change goes, one way to do it would be by using a function decorator to define when the function of the decorator it is applied to will get called based on the value of its first argument. This will eliminate the need for a long series of if-statements in your _on_value_change_event() method — because it can be replaced with a single call to the named decorated function.

I got the idea for implementing a decorator to achieve this from the article Learn About Python Decorators by Writing a Function Dispatcher, although I have implemented it in a slightly different manner using a class (instead of a function with nested-functions), which I think is a little "cleaner". Also note how the decorator itself uses an internal dictionary named registry to do the store information about and to do the actual dispatching — another "pythonic" way to do things.

Again, here's what I mean:

class dispatch_on_value:
    """ Value-dispatch function decorator.

    Transforms a function into a value-dispatch function, which can have
    different behaviors based on the value of its first argument.

    See: http://hackwrite.com/posts/learn-about-python-decorators-by-writing-a-function-dispatcher
    """
    def __init__(self, func):
        self.func = func
        self.registry = {}

    def dispatch(self, value):
        try:
            return self.registry[value]
        except KeyError:
            return self.func

    def register(self, value, func=None):
        if func is None:
            return lambda f: self.register(value, f)
        self.registry[value] = func
        return func

    def __call__(self, *args, **kw):
        return self.dispatch(args[0])(*args, **kw)

@dispatch_on_value
def react_to_param(idx):  # Default function when no match.
    print(f'In default react_to_param function for idx == {idx}.')

@react_to_param.register(15)
def _(idx):
    print('In react_to_param function registered for idx 15')

@react_to_param.register(16)
def _(idx):
    print('In react_to_param function registered for idx 16')

@react_to_param.register(17)
def _(idx):
    print('In react_to_param function registered for idx 17')

# Test dispatching.
for idx in range(14, 19):
    react_to_param(idx)  # Only this line would go in your `_on_value_change_event()` method.

Here's the output produced that shows it works. Note that there wasn't a function registered for every possible value, in which the the default function gets called.

In default react_to_param function for idx == 14.
In react_to_param function registered for idx 15
In react_to_param function registered for idx 16
In react_to_param function registered for idx 17
In default react_to_param function for idx == 18.
NickS1
  • 496
  • 1
  • 6
  • 19
martineau
  • 119,623
  • 25
  • 170
  • 301
  • Thanks this is a great suggestion, even though you stated you could not help me with my design. The decorater seems really neat. I'll try it out later today. Thanks again for your help :) – Mats de Waard Oct 04 '21 at 09:16
0

You should use something better than a CSV. A YAML file, for example. https://tutswiki.com/read-write-yaml-config-file-in-python/

alec_djinn
  • 10,104
  • 8
  • 46
  • 71
  • Yes, I was wondering about using JSON (should be similar to YAML right?) instead of CSV as well, but I don't really see how this would solve my issue with linking a seperate "on_value_change" event to each entry of my configurable parameters (seperate from if these parameters are saved in a YAML or CSV file). Do you mean by adding a function name to each entry of my configurable parameters, and than excute that function name? – Mats de Waard Sep 29 '21 at 11:41
  • 1
    Once you parse a YAML or JSON file you can refer to the variables directly, you don't need to use indices anymore. The rest is up to you, there is nothing wrong in using a bunch of if/else statemements. You may even try using the new Structural Pattern Matching available from Python 3.10. – alec_djinn Sep 29 '21 at 12:17