iiasa / ixmp4

A data warehouse for high-powered scenario analysis in the domain of integrated assessment of climate change and energy systems modeling
https://docs.ece.iiasa.ac.at/ixmp4
MIT License
10 stars 6 forks source link

Change plural form of column names to singular #102

Open glatterf42 opened 1 month ago

glatterf42 commented 1 month ago
          Is this on purpose that these are plural (values, units)?

_Originally posted by @danielhuppmann in https://github.com/iiasa/ixmp4/pull/101#discussion_r1673363628_

As mentioned above, many optimization objects at the moment expect to retrieve hardcoded column names such as elements, values, and levels, as appropriate. These names should be changed to their singular form in the DB layer (please correct me if this is wrong, @danielhuppmann). This could happen before the respective PRs are merged or with a separate cleanup immediately afterwards.

danielhuppmann commented 1 month ago

Thanks for creating this issue @glatterf42. I looked a bit more closely at the previous ixmp API vs. the new implementation, and I think the changes are not that big...

The main issue...

The methods ixmp.Scenario.add_par(name, data) takes and the ixmp.Scenario.par(name) returns a dataframe with columns value and unit (and variable/equation with columns level and marginal).

I assume that many users have elaborate scripts for working with these dataframe in pre- or postprocessing, so in order to keep disruption during migration to ixmp4 minimal, we have to maintain this behavior with the methods ixmp4.Run.optimization.Parameter.add() and ixmp4.Run.optimization.Parameter.data.

Methods that were introduced only in ixmp4 like ixmp4.Run.optimization.IndexSet.elements or ixmp4.Run.optimization.Parameter.units can remain in plural form or be converted to singular, not sure which is more intuitive...

Not sure which strategy is better, adapting the PRs or doing a follow-up PR...

glatterf42 commented 1 month ago

In principle, this would probably be something that the shim layer that @khaeru suggested could take care of. However, we can of course adjust the names of the columns in the dataframes for historical consistency. I would prefer to do this as a clean-up PR, I think, because it was a bit tedious to cherry-pick all commits for #101 since they all required very similar resolutions of merge conflicts and I would like to avoid repeating this. No one from the team is likely to just suddenly explore the tutorial, either, so I would start a clean up PR as soon as the data-model-PRs and the tutorial is merged and advertise only once that is complete. This PR would

For things like Indexset.elements, I would intuitively opt to keep the plural form because if I run Indexset.element, I would not expect to receive a list.

danielhuppmann commented 1 month ago

Sounds good to me to do the consistency-renaming as a follow-up PR, and agree to keep the elements as plural.