Bugfix/get geometries extent antimeridian #524
Conversation
|
Working on fixing the failed checks... |
|
Ok so one last test is still failing: It looks as if the PR slightly changed the extent of some countries used in this test. I think this might be due to the fact that the filtering of the shapes in natural earth has been slightly adjusted within this PR. Does anybody know who wrote these tests? In my opinion we can update the expected value of these tests, so they pass. However I need someone else's opinion to confirm as I don't want to oversee something that also could be an issue. |
tovogt
force-pushed
the
bugfix/get-geometries-extent-antimeridian
branch
from
ed63394 to
a4344d8
Compare
|
From my perspective, this is ready to be merged. I just added some code clean-up for the linter, but the rest is fine.
The tests were "broken" from the start because they passed the keyword arguments as positional arguments and the result was that the |
|
🙌 |
* Ensure get_country_geometries deals correctly with extent crossing the anti-meridian * get_land_gemoetry to rely on get_country_geometries to avoid redundancies * Ensure coord_on_land properly deals with extent of land_geom and lon * Adding a simple unit test for coord_on_land with points beyond the anti-meridian * Fix in case land_geom is empty (no land within extent) * provide keyword arg in unit test test_get_land_geometry_country_pass * u_coord: some refactoring and linting * u_coord: fix tests for get_land_geometry * coordinates: shapely.geometry.Polygon still referenced in `points_to_raster` * test_coordinates: complementary testcases for get_country_geometries Co-authored-by: Thomas Vogt <[email protected]> Co-authored-by: emanuel-schmid <[email protected]>
|
Thanks a lot @tovogt and @emanuel-schmid for reviewing and merging! |
Changes proposed in this PR:
climada.util.coordinates.get_land_geometrydeals correctly with extent. Previously, if an extent going beyond [-180, +180] range in longitude the geometry would be cut (e.g. passingextent=(160, 200, -50,50)would actually return the land geometry only for the extent(160, 180, -50, 50).get_country_geometry.coord_on_landto ensure longitude coordinates are also properly dealt with.This PR fixes issue #495.
A first test has been implemented but this can be completed.
Pull Request Checklist
develop)