Closed SabineHaas closed 2 years ago
Thanks for your work! Looks good. Some comments:
* Some of the constants names do not make a lot of sense outside of the context of the script, e.g. COL_SELECT, VAR_NAME, SCENARIO_KEY, NAME. Maybe renaming could help.
Okay. However, I would suggest to keep the names similar. e.g. var_name, scenario_key and so on to be able to associate them with the column names.
* The usage of all caps is not consistent. Some constants in the config are all caps, others not.
True. There's a mix already in #243. We should decide for on way.
Did you perform a test run?
No, I get an error, also when running your PR #243. I have realized just now that the tests of #243 in the CI do not fail. So there must be something wrong with my installation. My suggestion: take over this PR, you can use all the constants I have added.
Open TODOs:
See #243