Skip to content

Bugfix/54 issue with inner outer holes sequence#55

Merged
rapkin merged 5 commits into
masterfrom
bugfix/54-issue-with-inner-outer-holes-sequence
Oct 31, 2025
Merged

Bugfix/54 issue with inner outer holes sequence#55
rapkin merged 5 commits into
masterfrom
bugfix/54-issue-with-inner-outer-holes-sequence

Conversation

@rapkin

@rapkin rapkin commented Oct 31, 2025

Copy link
Copy Markdown
Collaborator

Description

Fixed issue #54 where OSM relations with non-consecutive
role ordering would incorrectly generate multiple outer
polygons instead of a single connected boundary.

Problem: When an OSM multipolygon relation had members
ordered as outer-outer-inner-outer (with outer ways split by
inner ways), the consecutive grouping logic would create
separate outer groups, resulting in fragmented polygons.

Solution: When multiple outer groups exist after consecutive
grouping, the fix attempts to merge all outer lines
together. The merged result is only used if it creates
exactly 1 polygon (indicating the segments actually
connect). This preserves correct behavior for complex cases
like Baarle-Nassau (issue #35) where separate outer groups
are intentional.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test addition or update

Related Issues

Fixes #54

Changes Made

  • Added logic in _convert_shapes_to_multipolygon() to detect
    and merge disconnected outer groups when they form a single
    connected polygon
  • Added test case test_issue_54_staffordshire_multipolygon
    with real-world OSM data (relation 195444 - Staffordshire
    county boundary)
  • Implementation is concise (~15 lines of additional logic)
    and maintains backward compatibility

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Tested manually (describe below)

Manual Testing Steps:

  1. generated geojson with osmtogeojson and visualised
  2. generated geojson with current version
  3. compared visually

Checklist

  • My code follows the code style of this project (ran ruff format and ruff check)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Screenshots (if applicable)

image

Additional Notes

@rapkin rapkin merged commit 4f4ad80 into master Oct 31, 2025
20 checks passed
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.

[BUG] OSM relation with outer and inner where outer segments are ordered after inner segments converts to multiple outers

1 participant