OSeMOSYS / otoole

OSeMOSYS Tools for Energy
https://otoole.readthedocs.io
MIT License
23 stars 17 forks source link

How to handle default values #139

Closed trevorb1 closed 1 year ago

trevorb1 commented 1 year ago

Question

This may be more of a question than an issue, but do you think we should clarify how users can modify default values in the planned v1.0 release? Specifically, should we remove all references to the default_values.csv and add DeprecationErrors as this information is now in the config.yaml?

Explanation

My main concern is that there are still a few references to the default_values.csv file in the code (see snippets below). This could be confusing if a user tries to modify this file, does not receive errors/warnings from otoole, but then their default values in the OSeMOSYS datafile are not updated. This is because the default values are now read in from the config file and should be modified there.

Proposed Solution

Similar to whats suggested in issue #135, I propose we remove all references to default_values.csv and raise a deprecation error if a default_values.csv is found. The error statement could be something like OtooleDeprecationError: Remove default_values.csv and define all default values in the configuration file

Other Info

Here are the references to default_values.csv I found in the code:

ReadDatapackage Class

This one likely won't be an issue after implementing issue #135

https://github.com/OSeMOSYS/otoole/blob/c4ea4ddcf5262ba94ee08a6680391cf30aee5546/src/otoole/read_strategies.py#L211-L220

Utils extract_config(schema) function

Only used in the ReadDatapackage.read(...) function

https://github.com/OSeMOSYS/otoole/blob/c4ea4ddcf5262ba94ee08a6680391cf30aee5546/src/otoole/utils.py#L79-L89

Write CSV Class

https://github.com/OSeMOSYS/otoole/blob/c4ea4ddcf5262ba94ee08a6680391cf30aee5546/src/otoole/write_strategies.py#L197-L204

Write DataPackage Class

Again, probably won't be an issue after implementing issue #135

https://github.com/OSeMOSYS/otoole/blob/c4ea4ddcf5262ba94ee08a6680391cf30aee5546/src/otoole/write_strategies.py#L225-L239

willu47 commented 1 year ago

I agree with the proposed solution. Thanks for the very clear write up.

trevorb1 commented 1 year ago

closed with PR #143