IAMconsortium / pyam

Analysis & visualization of energy & climate scenarios
https://pyam-iamc.readthedocs.io/
Apache License 2.0
221 stars 115 forks source link

Switch IIASA-Connection `default` argument #733

Closed danielhuppmann closed 1 year ago

danielhuppmann commented 1 year ago

Please confirm that this PR has done the following:

Description of PR

In anticipation of the release of the re-implemented ixmp package, this PR changes the API of the iiasa.Connection methods to resemble other filter options (and the forthcoming ixmp API).

Currently, a user could be confused that the query

iiasa.Connection("integration-test").index(default=False)

will return a table of runs that have both True and False in the is_default column.

Going forward, a user has to do is_default=None to get all scenarios (default and non-default), while is_default=True will return only the default scenarios (as currently implemented by default=True).

The new ixmp package will also support is_default=False to return only the non-default scenarios - the current ixmp API does not support that option.

codecov[bot] commented 1 year ago

Codecov Report

Merging #733 (28ce008) into main (13a1b93) will increase coverage by 0.0%. The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main    #733   +/-   ##
=====================================
  Coverage   95.1%   95.1%           
=====================================
  Files         59      59           
  Lines       6044    6057   +13     
=====================================
+ Hits        5748    5761   +13     
  Misses       296     296           
Impacted Files Coverage Δ
pyam/iiasa.py 87.3% <100.0%> (+0.5%) :arrow_up:
pyam/logging.py 64.8% <100.0%> (ø)
tests/test_iiasa.py 97.8% <100.0%> (ø)

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

danielhuppmann commented 1 year ago

Thanks for the clarification, @meksor, will update the PR

danielhuppmann commented 1 year ago

After some thinking, I went back to simply renaming the kwarg from "default" to "default_only" for forward-compatibility (with a deprecation-warning until release 2.0). The "is_default"-option (True, False, None) will not be available in the current (soon-to-be-legacy) API, so it's not necessary to add that here.