[qsr_lib] Suggestion: Lose the 'make' function

[cdondrup]

This is a follow up of #133. Currently the make function is where all the magic happens but it also hosts a lot of duplicate code. Hence, this suggestion:

Have one make function in the QSR_Abstractclass which calls a few well structured and purpose bound customised functions

The pseudo python code for this could look like this:

def make(self, *args, **kwargs):
        input_data, timestamps = set_input_data(**kwargs)
        ret = World_QSR_Trace(qsr_type=self._unique_id)
        parameters_dict = get_parameters(**kwargs)
        for p in qsrs_for:
            if check_duplicate(o1_name, o2_name):
                ret.add_qsr(lookup_function(o1_name, o2_name))
            else:
                ret.add_qsr(compute_qsr(parameters_dict, ...))
        ret = post_process(ret)
        return ret

Here are the main functions used:

set_input_data Quite self explanatory

def set_input_data(**kwargs):
    input_data = kwargs["input_data"]
    timestamps = input_data.get_sorted_timestamps()
    return input_data, timestamps

get_parameters This function returns all the parameters necessary like qsr_relations_and_values or quantisation_factor in a dictionary and would be abstract so people have to implement it. But this could as well be standardised once all the deprecated stuff is gone.

check_duplicate Checks if the inverse object combination has been calculated already and return True if so. Default implementation would be:

def check_duplicate(o1_name, o2_name):
    return False

this way you can override it to save time but you don't have to.

lookup_function This function turns the result for o1,o2 into o2,o1. Default implementation:

def lookup_function(o1_name, o2_name):
    raise NotImplementedError

This way the user will be forced to override it if check_duplicate is overridden. Otherwise this will never be called and the make function just calculates all the possible combinations.

compute_qsr is where all the magic goes and what actual computes your QSR. This would be an abstract function, so people have to implement it.

post_process is for everything you have to do with the result after it has been calculated, e.g. the timstamp rectification of QTC. The default implementation would be:

def post_process(qsr):
    return qsr

The reasoning behind all this, currently the make function has a lot of overhead and is not well structured or especially easy to read. By splitting it up into two mandatory (and a few optional functions to speed up calculation or do post processing) this would be clearer and also easier to maintain. It would also enforce more standardisation which is always a good thing.

Edit: This change wouldn't break anything as the make function is currently overridden in every of the QRSs, therefore, the transition doesn't have to be abrupt.

Opinions?

[pet1330]

I like it 😄

[gatsoulis]

Yes, leave it with me as it is related to #118 .