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

Geogrid fix #340

Closed dbkhout closed 8 months ago

dbkhout commented 8 months ago

While only used for visualization, the get_geogrid and reproject_geodata functions seem to be slightly off.

The functions use np.linspace to retrieve the lower boundary for each pixel based on the number of pixels and the extent of the image (e.g. x1, x2). In doing so, x2 should be excluded from the interval as it represents the upper boundary of the image.


Illustration:

np.linspace(3, 10, 7) returns 3., 4.16666667, 5.33333333, 6.5, 7.66666667, 8.83333333, 10. while 3., 4., 5., 6., 7., 8., 9. is expected.

np.linspace(3, 10, 7, endpoint=False) does return the expected result.


Note that this has been accounted for correctly (yet with different approaches) in other pysteps functions:

https://github.com/pySTEPS/pysteps/blob/8dca14afbf74f47072a810317a8bddefa33e3e60/pysteps/utils/dimension.py#L410

https://github.com/pySTEPS/pysteps/blob/8dca14afbf74f47072a810317a8bddefa33e3e60/pysteps/io/exporters.py#L336

codecov[bot] commented 8 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (71fa354) 83.33% compared to head (c8103a7) 83.32%. Report is 4 commits behind head on master.

Files Patch % Lines
pysteps/visualization/utils.py 50.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #340 +/- ## ========================================== - Coverage 83.33% 83.32% -0.01% ========================================== Files 161 161 Lines 12358 12356 -2 ========================================== - Hits 10298 10296 -2 Misses 2060 2060 ``` | [Flag](https://app.codecov.io/gh/pySTEPS/pysteps/pull/340/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/340/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pySTEPS) | `83.32% <50.00%> (-0.01%)` | :arrow_down: | 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.

dnerini commented 8 months ago

hi @dbkhout thanks for the PR! Tests are failing for macos and windows, but it's probably unrelated from your changes. I'll have a closer look asap and will let you know

dnerini commented 8 months ago

@dbkhout could you please merge the latest version of pysteps/master in your branch? This should fix the errors in the CI pipelines

dbkhout commented 8 months ago

You're welcome, thanks for the quick reaction!