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

Move to openscm conventions #154

Merged
merged 29 commits into from Oct 19, 2018
Merged

Move to openscm conventions #154

merged 29 commits into from Oct 19, 2018

Conversation

znicholls
Copy link
Collaborator

@znicholls znicholls commented Oct 16, 2018

Pull request

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Example added (either to an existing notebook or as a new notebook, where applicable)
  • Description in CHANGELOG.rst added
  • check that old API still returns a dict (moved to Move to one backend, not two #161)
  • new style returns new style Dataframe (moved to Move to one backend, not two #161)

Adding to CHANGELOG.rst

Please add a single line in the changelog notes similar to one of the following:

- (`#XX <http://link-to-pr.com>`_) Added feature which does something
- (`#XX <http://link-to-pr.com>`_) Fixed bug identified in (`#XX <http://link-to-issue.com>`_)

@znicholls znicholls mentioned this pull request Oct 16, 2018
6 tasks
@rgieseke
Copy link
Member

Column headers convention:
Lowercase, underscore, follow Pyam

Variable and acronym naming convention:
use (and enhance) ghgrenamer (https://github.com/openclimatedata/ghgrenamer), "Emissions|N2O|MAGICC AFOLU"

'B' becomes" MAGICC AFOLU"
'I' becomes "MAGICC Fossil and Industrial"

Regional split:
World|Northern Hemisphere|Land
World|R5...
(copy Pyam)

@znicholls
Copy link
Collaborator Author

Regional split:
World|Northern Hemisphere|Land

'World' rather than 'Global' here is jarring but it's better to be consistent with Pyam than precious

@znicholls
Copy link
Collaborator Author

from earlier discussions:

  • move to long format only (in preparation for using pyam capability)

@znicholls
Copy link
Collaborator Author

@rgieseke Atmospheric Concentrations|CO2 or Concentrations|CO2, I think the argument for the former is in case we one day introduce an ocean pH module to MAGICC and need Ocean concentrations or similar

@znicholls znicholls force-pushed the move-to-openscm-conventions branch 2 times, most recently from f3c4a70 to d3be60a Compare October 16, 2018 23:28
@codecov-io
Copy link

codecov-io commented Oct 18, 2018

Codecov Report

Merging #154 into master will increase coverage by 0.4%.
The diff coverage is 98.19%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #154     +/-   ##
=========================================
+ Coverage   96.33%   96.73%   +0.4%     
=========================================
  Files           4        5      +1     
  Lines         981     1132    +151     
  Branches      134      174     +40     
=========================================
+ Hits          945     1095    +150     
+ Misses         22       20      -2     
- Partials       14       17      +3
Impacted Files Coverage Δ
pymagicc/utils.py 100% <100%> (ø)
pymagicc/io.py 96.79% <97.97%> (+0.25%) ⬆️
pymagicc/definitions/__init__.py 98.31% <98.23%> (-1.69%) ⬇️

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 481d467...4220418. Read the comment docs.

Questions to answer:

- column header convention (I'd follow pyam so lowercase everything)
- variable naming convention (I'd be tempted to copy pyam so captialise words
  e.g. Emissions|CO2|Land-use & Agriculture)
- suffixes to use re 'I' and 'B'
- regional split to use re land/ocean, NH/SH
- acronyms to use e.g. "BC" or "Black Carbon"
Almost there, need to fix replacements generators and units writing
Tomorrow

- speed up
- tidy up
- merge
@znicholls znicholls changed the title WIP: Move to openscm conventions Move to openscm conventions Oct 19, 2018
@znicholls
Copy link
Collaborator Author

@rgieseke this one is pretty massive. Tried to keep it just to the minimum I needed, unfortunately that's quite a lot...

CHANGELOG.rst Outdated Show resolved Hide resolved
Copy link
Member

@rgieseke rgieseke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Makefile Show resolved Hide resolved
pymagicc/io.py Outdated Show resolved Hide resolved
docs/index.rst Show resolved Hide resolved
@znicholls znicholls merged commit 3d0f09f into master Oct 19, 2018
@znicholls znicholls deleted the move-to-openscm-conventions branch October 19, 2018 10:37
@znicholls znicholls mentioned this pull request Oct 26, 2018
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.

None yet

3 participants