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

Add a unit_mapping attribute to show a variable -> unit dictionary #548

Merged
merged 5 commits into from
Jun 22, 2021

Conversation

danielhuppmann
Copy link
Member

@danielhuppmann danielhuppmann commented Jun 22, 2021

Please confirm that this PR has done the following:

  • Tests Added
  • Documentation Added
  • Name of contributors Added to AUTHORS.rst
  • Description in RELEASE_NOTES.md Added

Description of PR

This PR adds an attribute unit_mapping to return a dictionary {variable: unit or list of units}. This is intended to replace the current function variables(include_units=True).

For illustration, the IamDataFrame

model scenario region variable unit 2005 2010
model_a scen_a World Primary Energy EJ/yr 1 6
model_a scen_a World Primary Energy|Coal EJ/yr 0.5 3
model_a scen_b World Primary Energy GWh/yr 2 7

will return

df.unit_mapping
> {"Primary Energy": ["EJ/yr", "GWh/yr"], "Primary Energy|Coal": "EJ/yr"}

Unique unit for a variable?

Note that it may be preferable to require that an IamDataFrame has a unique unit for each variable (see the related discussion at #338). Currently, several units are allowed, so this PR simply maintains the current behavior.

@danielhuppmann danielhuppmann self-assigned this Jun 22, 2021
@danielhuppmann danielhuppmann marked this pull request as ready for review June 22, 2021 12:18
@danielhuppmann
Copy link
Member Author

I know that everyone is super-busy, but @gidden @znicholls @coroa, if you have a minute for a quick look just about the suggested API change, it would be very much appreciated...

@codecov
Copy link

codecov bot commented Jun 22, 2021

Codecov Report

Merging #548 (740a721) into main (7d83a98) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #548     +/-   ##
=======================================
+ Coverage   93.4%   93.6%   +0.1%     
=======================================
  Files         48      48             
  Lines       5288    5299     +11     
=======================================
+ Hits        4942    4961     +19     
+ Misses       346     338      -8     
Impacted Files Coverage Δ
tests/test_unfccc.py 100.0% <ø> (ø)
pyam/core.py 92.7% <100.0%> (+<0.1%) ⬆️
tests/test_core.py 100.0% <100.0%> (ø)
tests/test_datareader.py 82.6% <0.0%> (+13.0%) ⬆️
pyam/datareader.py 100.0% <0.0%> (+50.0%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d83a98...740a721. Read the comment docs.

@gidden
Copy link
Member

gidden commented Jun 22, 2021

lgtm! I have no strong feelings about the name, given that it is not simply a list like variables.

@gidden gidden merged commit 6435898 into IAMconsortium:main Jun 22, 2021
@znicholls
Copy link
Collaborator

Nice nice

@danielhuppmann danielhuppmann deleted the attrs/unit_mapping branch June 23, 2021 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants