aspectumapp / osm2geojson

Convert OSM and Overpass XML/JSON to GeoJSON
MIT License
97 stars 15 forks source link

Any reason not to return OSM multipolygons as simple Polygons when possible and valid? #33

Open dericke opened 2 years ago

dericke commented 2 years ago

Many features that count as multipolygons under the OSM definition are valid Polygons under the geojson definition: anything with a single exterior member and all other members located inside could be valid Polygons.

I notice that even when an OSM multipolygon relation would be a valid Polygon feature in geojson, it is returned as single-member MultiPolygon instead. This adds complexity; for example, reorienting a MultiPolygon so that it follows the right-hand rule requires several lines of code to iterate over the members, whereas a Polygon only requires a single line of code to do the same. Is there a benefit to always returning a MultiPolygon that makes this added complexity worthwhile?

rapkin commented 2 years ago

In code we are working with multipolygons (in general case). It possible to detect polygons (and convert) in function multipolygon_relation_to_shape after line 482 https://github.com/aspectumapp/osm2geojson/blob/068b83afe19cff1ae15b9efc2a9ff5a9be8928e7/osm2geojson/main.py#L482 but I see that mapping function generates valid Polygon even if you pass shapely.MultiPolygon, here example https://github.com/aspectumapp/osm2geojson/blob/068b83afe1/tests/data/map.geojson?short_path=b808ffd#L52

The only place when this change can be helpful - when you are working with objects produced by json2shapes/xml2shapes methods. And I think we can try to change multipolygon_relation_to_shape method. But we have high probability of breaking change (so I'll update minor version number).

dericke commented 2 years ago

The case I ran into was trying to convert polygons to follow the right-hand rule: because they are output as MultiPolygons, it took a few more lines of code to do the conversion compared to simple Polygons.

I actually meant to open a separate issue for the polygon winding order, then forgot, so I'll do that now.

rapkin commented 2 years ago

yes, I need more details to understand problem with right-hand rule. Maybe this line can be helpful for you (unfortunately I don't remember the purpose of it) https://github.com/aspectumapp/osm2geojson/blob/master/osm2geojson/main.py#L222

rapkin commented 2 years ago

So what about this now? It should be easy to return Polygons in json2shapes and xml2shapes methods (here example https://github.com/aspectumapp/osm2geojson/blob/return-polygon-shape/osm2geojson/main.py#L483). But this is also breaking change