Skip to content

Conversation

@ValentinGebhart
Copy link
Collaborator

@ValentinGebhart ValentinGebhart commented Mar 4, 2025

Changes proposed in this PR:

  • Fixed bug in util.coordinates.bounding_box_from_countries, if the country is a polygon and not a multipolygon, e.g., isocode = 'CHE'.
  • added the polygon case to the unit tests

PR Author Checklist

PR Reviewer Checklist

@ValentinGebhart ValentinGebhart changed the title fixed bug in bounding_box_from_countries hotfix: bug in bounding_box_from_countries Mar 4, 2025
Copy link
Member

@peanutfun peanutfun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The case is obviously not covered by a test, or else this would have been noticed before. The linter also complains about that. Could you add a test involving a polygon instead of a multipolygon? But please check first if you cannot simply iterate over the coords attribute of MultiPolygon, too.

Copy link
Member

@peanutfun peanutfun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks very nice now! I approve under the condition that you remove (now unused) the import of Polygon, as Jenkins complains. Then you can merge!

@ValentinGebhart ValentinGebhart merged commit 17fa00e into develop Mar 6, 2025
15 checks passed
@ValentinGebhart ValentinGebhart deleted the hotfix_bounding_box_from_countries branch March 6, 2025 14:10
emanuel-schmid added a commit that referenced this pull request Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants