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

Facilitate region-aggregation with inconsistent model scenario region time #792

Conversation

danielhuppmann
Copy link
Member

@danielhuppmann danielhuppmann commented Oct 17, 2023

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 improves the handling of region-aggregation when variables and weights have an inconsistent index.

Now, if weights are missing, a ValueError is raised with (the head of) a list of missing rows. If variable data is missing, a warning is written to the log with (the head of) a list of missing rows.

closes #558

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #792 (93e39a6) into main (f023ec3) will increase coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main    #792     +/-   ##
=======================================
+ Coverage   94.5%   94.7%   +0.1%     
=======================================
  Files         62      62             
  Lines       5966    5996     +30     
=======================================
+ Hits        5643    5681     +38     
+ Misses       323     315      -8     
Files Coverage Δ
pyam/aggregation.py 99.2% <100.0%> (+<0.1%) ⬆️
tests/test_feature_aggregate.py 99.0% <100.0%> (+0.1%) ⬆️
tests/test_feature_growth_rate.py 100.0% <100.0%> (ø)
tests/test_io.py 100.0% <100.0%> (ø)

... and 2 files with indirect coverage changes

@danielhuppmann danielhuppmann marked this pull request as ready for review October 18, 2023 05:51
@danielhuppmann
Copy link
Member Author

I vaguely recall that @orichters expressed interest in a more flexible handling of the region-aggregation-with-weights...?

@orichters
Copy link

I vaguely recall that @orichters expressed interest in a more flexible handling of the region-aggregation-with-weights...?

I do not 😃. Maybe @Renato-Rodrigues, if it was someone from the REMIND team?

Copy link
Contributor

@phackstock phackstock 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.
Can #754 be closed then since you're no longer raising an issue if weights are a superset of the data?

@danielhuppmann
Copy link
Member Author

Right, this PR (together with #787) was a more structured second attempt at #754 to have a clean separation between clean-up and new features...

@danielhuppmann danielhuppmann merged commit 5b2f3b6 into IAMconsortium:main Oct 19, 2023
11 checks passed
@danielhuppmann danielhuppmann deleted the feature/aggregate-region-weights-inconsistent branch October 19, 2023 08:53
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.

df.aggregate_region() when inconsistent index (e.g. years)
3 participants