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 weighted quantiles #686

Merged
merged 13 commits into from
Dec 15, 2022
Merged

add weighted quantiles #686

merged 13 commits into from
Dec 15, 2022

Conversation

gidden
Copy link
Member

@gidden gidden commented Aug 12, 2022

Description of PR

Adds a df.quantile() function which calculates (possibly weighted) quantiles for a variable across a population of models and scenarios.

If this feature is useful for others in the community, I'm happy to improve the PR, add a tutorial, etc.

cc @chrisroadmap

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

TODOS - issues

  • support custom names (currently put into model)
  • more flexible weights argument

Adding to RELEASE_NOTES.md (remove section after adding to RELEASE_NOTES.md)

Please add a single line in the release notes similar to the following:

- (#XX)[http://link-to-pr.com] Added feature which does something

@gidden gidden requested a review from danielhuppmann August 12, 2022 08:23
@codecov
Copy link

codecov bot commented Aug 12, 2022

Codecov Report

Merging #686 (84c69ea) into main (947d8c4) will increase coverage by 0.0%.
The diff coverage is 93.2%.

@@          Coverage Diff          @@
##            main    #686   +/-   ##
=====================================
  Coverage   94.8%   94.9%           
=====================================
  Files         58      59    +1     
  Lines       5899    5990   +91     
=====================================
+ Hits        5598    5690   +92     
+ Misses       301     300    -1     
Impacted Files Coverage Δ
pyam/iiasa.py 86.7% <ø> (ø)
tests/test_iiasa.py 97.8% <ø> (-0.1%) ⬇️
tests/test_io.py 98.4% <85.7%> (-1.6%) ⬇️
pyam/compute.py 96.2% <91.6%> (-3.8%) ⬇️
pyam/core.py 95.2% <100.0%> (+0.6%) ⬆️
pyam/utils.py 91.7% <100.0%> (ø)
tests/test_feature_quantiles.py 100.0% <100.0%> (ø)
tests/test_ops.py 100.0% <0.0%> (ø)
tests/test_core.py 100.0% <0.0%> (ø)
tests/test_feature_validation.py 100.0% <0.0%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@danielhuppmann
Copy link
Member

Thanks @gidden, this sounds really useful!

A few comments:

  1. I developed a Statistics module for the SR15, see the docs and a real-world use case here. The main advantage of the Statistics class is that you often want to do quantiles on groups of scenarios (e.g., the temperature categorization). If I understand the current implementation correctly, the method will always compute quantiles grouped by scenario names indexed by model, region and variable - we should probably anticipate a more flexible approach.
  2. If you proceed with implementation in this way, I suggest to add that as method like IamDataFrame.compute.quantiles() to avoid too many methods directly on the IamDataFrame class. See the docs.

@gidden
Copy link
Member Author

gidden commented Oct 12, 2022

@danielhuppmann - FYI - I am seeing test failures in test_datareader.py not related to this PR.

I'm also hoping to clean this up, time permitting, in the next days.

@danielhuppmann
Copy link
Member

@danielhuppmann - FYI - I am seeing test failures in test_datareader.py not related to this PR.

But all checks except one passed, so I guess that the Woldbank-server just had a hick-up in that second. Re-running the failed test might do the trick...

Copy link
Member

@danielhuppmann danielhuppmann 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, a few minor suggestions inline!

pyam/compute.py Show resolved Hide resolved
pyam/compute.py Outdated Show resolved Hide resolved
pyam/core.py Outdated Show resolved Hide resolved
pyam/compute.py Outdated Show resolved Hide resolved
Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

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

Three more minor fixes, then good to be merged. Thanks for adding a tutorial!

tests/test_feature_quantiles.py Outdated Show resolved Hide resolved
tests/test_feature_quantiles.py Outdated Show resolved Hide resolved
pyam/compute.py Outdated Show resolved Hide resolved
gidden and others added 3 commits December 14, 2022 18:21
Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>
Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>
Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>
@gidden
Copy link
Member Author

gidden commented Dec 14, 2022

all committed, should be g2g @danielhuppmann ! Thanks for the review!

@danielhuppmann
Copy link
Member

Release notes are still missing - pretty please!

@gidden
Copy link
Member Author

gidden commented Dec 15, 2022

Release notes updated and issues made @danielhuppmann

@danielhuppmann
Copy link
Member

Thanks @gidden!

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.

2 participants