search

pySTEPS / pysteps

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

Fix: make sure that precip_models variable has the correct length #368

Closed RubenImhoff closed 3 weeks ago

RubenImhoff commented 1 month ago

Fix to ensure that the _check_inputs function in /blending/steps.py looks for the correct precip_models length when a list of timesteps instead of an integer is provided.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.82%. Comparing base (3121cbf) to head (64c8250). Report is 7 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #368 +/- ## ========================================== + Coverage 83.50% 83.82% +0.31% ========================================== Files 159 160 +1 Lines 12564 12788 +224 ========================================== + Hits 10492 10719 +227 + Misses 2072 2069 -3 ``` | [Flag](https://app.codecov.io/gh/pySTEPS/pysteps/pull/368/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pySTEPS) | Coverage Δ | | |---|---|---| | [unit_tests](https://app.codecov.io/gh/pySTEPS/pysteps/pull/368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pySTEPS) | `83.82% <100.00%> (+0.31%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pySTEPS#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ladc commented 1 month ago

Thanks! I haven't used custom timesteps so far, so just to check -I thought it was allowed to have non-integer timesteps if you specify custom ones, so what happens in that case? (maybe requires a ceil?)

RubenImhoff commented 1 month ago

That is a good point, @ladc! I have not tested that (yet), but I can imagine that a ceil would solve that potential issue. Should we make sure that the number gets rounded up in that case to ensure that the non-integer timestep always falls within the total forecast length?

dnerini commented 1 month ago

should this be added to the hackathon?