pySTEPS / pysteps

Python framework for short-term ensemble prediction systems.
https://pysteps.github.io/
BSD 3-Clause "New" or "Revised" License
466 stars 168 forks source link

Make sure a skill array with the right dimensions is returned when multiple models are used #279

Closed RubenImhoff closed 2 years ago

RubenImhoff commented 2 years ago

This PR should fix #277. I have also adjusted the tests to pick up this issue in the future.

@mpvginde, could you check if this solves the issue for you?

codecov[bot] commented 2 years ago

Codecov Report

Merging #279 (6503b11) into master (464525f) will increase coverage by 0.01%. The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master     #279      +/-   ##
==========================================
+ Coverage   82.32%   82.34%   +0.01%     
==========================================
  Files         158      158              
  Lines       12130    12135       +5     
==========================================
+ Hits         9986     9992       +6     
+ Misses       2144     2143       -1     
Flag Coverage Δ
unit_tests 82.34% <88.88%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pysteps/blending/skill_scores.py 87.27% <50.00%> (ø)
pysteps/blending/steps.py 83.89% <100.00%> (+0.16%) :arrow_up:
pysteps/tests/test_blending_skill_scores.py 100.00% <100.00%> (ø)
pysteps/tests/test_blending_steps.py 99.13% <100.00%> (+0.01%) :arrow_up:

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 464525f...6503b11. Read the comment docs.

RubenImhoff commented 2 years ago

Will do so, thanks Daniele!

dnerini commented 2 years ago

Thanks Ruben for extending the tests. Do you want to wait for a feedback from @mpvginde before merging?

RubenImhoff commented 2 years ago

Thanks Ruben for extending the tests. Do you want to wait for a feedback from @mpvginde before merging?

Yes, perhaps good for double checking. :)

mpvginde commented 2 years ago

Hi @RubenImhoff and @dnerini , I just checked and the error is gone. Thanks for taking care of the issue!