Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support GeoJSON MultiPolygon #217

Closed
simon04 opened this issue Nov 13, 2016 · 5 comments
Closed

Support GeoJSON MultiPolygon #217

simon04 opened this issue Nov 13, 2016 · 5 comments
Labels

Comments

@simon04
Copy link
Contributor

simon04 commented Nov 13, 2016

@tyrasd in #206 (comment):

Well, actually, there are already a couple of "multipolygon" sources in the repo (for example the South Tyrol 2014, or the Tiris * imagery), but which we forgot about in the geojsonification process in #191. Instead, they continued to use the slightly odd (and incompatible to GeoJSON) convention where the rings in the coordinates array are all outer rings and inner rings are not allowed.

It's probably best to fix those all in one go and to apply an additional fix (very similar to yours also to fix the legacy json output).

For this particular layer this means that we can either wait until this is all fixed in the data processing before adding the source, or that we merge it using the old&incorrect coordinate scheme (because the PR as-is will probably break things for users of our legacy-json output).

@simon04
Copy link
Contributor Author

simon04 commented Nov 13, 2016

Change in convert_xml.py to support MultiPolygon:

diff --git a/scripts/convert_xml.py b/scripts/convert_xml.py
index f2a0b34..47ea57a 100644
--- a/scripts/convert_xml.py
+++ b/scripts/convert_xml.py
@@ -68,7 +68,7 @@ for source in sources:
     if geometry:
         bounds = ET.SubElement(entry, "bounds")

-        for ring in geometry['coordinates']:
+        for ring in geometry['coordinates'] if geometry['type'] != 'MultiPolygon' else [ring for rings in geometry['coordinates'] for ring in rings]:
             shape = ET.SubElement(bounds, "shape")
             for p in ring:
                 point = ET.SubElement(shape, "point")

@rbuffat
Copy link
Collaborator

rbuffat commented Jul 21, 2020

@andrewharvey @bhousel
MultiPolygons would be handy for many sources. Is there a reason that MultiPolygons are not allowed? Are there editors that would break? It looks as some sources use interior rings of Polygons to "emulate" MultiPolygons, which leads to invalid geometries.

@bhousel
Copy link
Member

bhousel commented Jul 21, 2020

MultiPolygons would be handy for many sources. Is there a reason that MultiPolygons are not allowed? Are there editors that would break? It looks as some sources use interior rings of Polygons to "emulate" MultiPolygons, which leads to invalid geometries.

No reason.. I'd actually prefer for the geometries be proper GeoJSON multipolygons.
And I've already gone thru and fixed them all.. twice 🙄

@rbuffat
Copy link
Collaborator

rbuffat commented Jul 21, 2020

No reason.. I'd actually prefer for the geometries be proper GeoJSON multipolygons.
And I've already gone thru and fixed them all.. twice

Then I assume you are happy about #857

@rbuffat
Copy link
Collaborator

rbuffat commented Feb 9, 2022

Implemented with #1450

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants