geoffroychaussonnet / script_to_monitor_Covid19

Python scripts to monitor Covid-19
BSD 3-Clause "New" or "Revised" License
1 stars 1 forks source link

Improve design of parameters (issue 9, WIP). #15

Closed jferard closed 4 years ago

jferard commented 4 years ago

I proceed step by step to split data from parameters.

jferard commented 4 years ago

Roadmap:

jferard commented 4 years ago

Roadmap:

jferard commented 4 years ago

Grouping parameters. I take the example of plot_versus_time.py: we have the line:

    plot_country(area, data, fitParam, quar_date, ax, field, vSmoothing,
                 evolutionType, yscale)

Two parameters are a natural group: area and quar_date (see #8). Most of the parameters are passed to evolution_country that returns the evolution of the country, given field and evolutionType, vSmoothing, ... Other parameters are used to draw the plot (ax, yscale).

The second most obvious group is evolutionType and vSmoothing, simply because not all evoutionTypes require a smoothing parameter. Hence, vSmoothing is a parameter evolutionType. I would see a class:

class SmoothedCurvature(EvolutionType):
    def __init__(window_length, polyorder):
          # assign parameters

    def evolution(self, cumulative_evolution):
         # override.

Creating a class for field would perhaps be overkill, but would remove those if... elif... elif ....

fitParam is a group by itself (a triplet), used to compute trends (could maybe be a class), and ax and y_scale are a fourth group.

jferard commented 4 years ago

A general remark: I noticed that there's a tension between the need to group global parameters of the script (e.g. in an json, ini, yaml, toml, ... file) and the "parameter objects" of the functions/methods. I hoped to split the dicts into plain variables and then to regroup them in parameter objects, but that's not so easy because that would create inconsistent parameter objects. I gave above a sketch of some small parameter objects, but it does not seem a good a idea to create (at one go) the "mega-parameter-object" that will contain all the parameters of the script, plus the inferred parameters (as the initial dicts did).

I propose to merge this (after a rebase) and to open new issues/PR to discuss and maybe create the parameter objects sketched above...