IAMconsortium / pyam

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

Fix/concat #665

Closed phackstock closed 2 years ago

phackstock commented 2 years ago

Please confirm that this PR has done the following:

Description of PR

closes #664.

The pyam.concat function now carries over the index of the merged meta table. Added a unit test to check under test_feature_append_and_concat.test_concat_non_standard_index. For documentation I added an additional line to the notes section of the pyam.concat function.

One open question from my side would be if there is more documentation than the added docstring required. My hunch would be that it's fine as is since the behavior is not really new but rather more working in line with user expectations.

codecov[bot] commented 2 years ago

Codecov Report

Merging #665 (f95c030) into main (c5e8f9c) will increase coverage by 0.0%. The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main    #665   +/-   ##
=====================================
  Coverage   94.5%   94.5%           
=====================================
  Files         59      59           
  Lines       5742    5747    +5     
=====================================
+ Hits        5431    5436    +5     
  Misses       311     311           
Impacted Files Coverage Δ
pyam/core.py 94.5% <100.0%> (ø)
tests/test_feature_append_concat.py 100.0% <100.0%> (ø)

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 c5e8f9c...f95c030. Read the comment docs.

phackstock commented 2 years ago

Thanks for the quick review @danielhuppmann. If the tests all run through I'd go ahead with the merge.

coroa commented 2 years ago

Thanks for that! I literally stumbled over it last night.

phackstock commented 2 years ago

Perfect coincidence then, happy to fix it.