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

Many notebooks #51

Merged
merged 26 commits into from
Mar 13, 2020
Merged

Many notebooks #51

merged 26 commits into from
Mar 13, 2020

Conversation

Rlamboll
Copy link
Collaborator

@Rlamboll Rlamboll commented Mar 9, 2020

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

Adding to CHANGELOG.rst

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

- (`#XX <https://github.com/znicholls/silicone/pull/XX>`_) Added feature which does something
- (`#XX <https://github.com/znicholls/silicone/pull/XX>`_) Fixed bug identified in (`#YY <https://github.com/znicholls/silicone/issues/YY>`_)

@codecov
Copy link

codecov bot commented Mar 9, 2020

Codecov Report

Merging #51 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #51   +/-   ##
=======================================
  Coverage   98.21%   98.22%           
=======================================
  Files          18       18           
  Lines         785      789    +4     
  Branches      165      166    +1     
=======================================
+ Hits          771      775    +4     
  Misses          8        8           
  Partials        6        6           
Impacted Files Coverage Δ
src/silicone/utils.py 100.00% <100.00%> (ø)

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 2bf97ec...601224c. Read the comment docs.

@Rlamboll Rlamboll requested a review from jkikstra March 9, 2020 15:53
@Rlamboll
Copy link
Collaborator Author

Rlamboll commented Mar 9, 2020

This is mostly just splitting up the notebooks, I have reworded and expanded the section on the rolling windows quantile cruncher though, with a significant new final section. There are also minor changes to the way we get data from IIASA in order to streamline the notebooks and to allow for an update.

@jkikstra
Copy link
Collaborator

Thanks for splitting these up.

I found two more things in notebook 05.

  • For "DecomposeCollectionTimeDepRatio" I do not get what the output is supposed to explain to me; both unit_consistant_db.variables(True) and results.variables() give the basically the same output. Could you clarify?
  • Under "calculate aggregate values" could you just add that 'using AR5 conversion factors' is done by setting the boolean use_AR4_data=False?

@Rlamboll
Copy link
Collaborator Author

Thanks @jkikstra, however I will need to merge #49 first. Maybe you can review the non-notebook parts of that too since @znicholls has a large to-do list.

@jkikstra
Copy link
Collaborator

I don't think I'll be able to do that this week still unfortunately.

@znicholls znicholls added this to the v0.2 milestone Mar 12, 2020
@znicholls
Copy link
Collaborator

this should be gtg once #49 is done (hopefully also not far off)

@Rlamboll Rlamboll merged commit 01c346b into master Mar 13, 2020
@Rlamboll Rlamboll deleted the many_notebooks branch March 13, 2020 00:43
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