Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overly ambitious groupby().sum()´ in rename()` #182

Closed
danielhuppmann opened this issue Jan 30, 2019 · 1 comment · Fixed by #202
Closed

Overly ambitious groupby().sum()´ in rename()` #182

danielhuppmann opened this issue Jan 30, 2019 · 1 comment · Fixed by #202

Comments

@danielhuppmann
Copy link
Member

Description of the problem

We identified a silly mistake in the IAMC 1.5°C Scenario Data due to the over-ambitious use of groupby().sum() in the rename() function implementation. See iiasa/ipcc_sr15_scenario_analysis#2 for details.

There were a number of variables with a typo, and when using rename() to correct that, I failed to check whether any variable without the typo also existed. This was the case for two variables, so the values are now double compared to what they should be.

Proposed solution

I would like to add a kwarg check_duplicate (default True) to the function, which raises an error message if any rename mapping leads to a conflict with the existing model-scenario-region-variable-unit-year index in the data. A user would need to consciously override the check to merge renamed and existing values as part of the rename. Idea: fail with a bang rather than return unexpected results.

The merging of data renamed to the same variable name should obviously always work.

@gidden, @OFR-IIASA, any comments?

@gidden
Copy link
Member

gidden commented Feb 26, 2019

Sounds reasonable to me. Probably easier to look at the implementation, as I don't 100% grok the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants