aspectumapp / osm2geojson

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

Failure to convert relation without outer ring defined to shape #41

Open sgoodm opened 1 year ago

sgoodm commented 1 year ago

A relation representing a simple polygon, but without an outer ring explicitly defined will fail. See below example for relation https://www.openstreetmap.org/relation/6127139

import osm2geojson

resp = osm2geojson.overpass_call('''
    [out:json];
    relation(6127139);
    out;
    >;
    out skel qt;
''')
osm2geojson.json2shapes(resp)

Breaking code is tied to https://github.com/aspectumapp/osm2geojson/blob/master/osm2geojson/main.py#L557 which will return None in the case where no outer ring has been defined for the relation.

Possibly tied to #38 and #33. Breaking changes implemented in 024c06b099bc41a0c734d8435c048116c2cc3055

Minimal working solution (for my use case, but may have broader undesirable implications) involves inserting below code here: https://github.com/aspectumapp/osm2geojson/blob/master/osm2geojson/main.py#L547

    if len(groups) == 1:
        return groups[0][1]
rapkin commented 1 year ago

@sgoodm Hello. Sorry for late response. I tested your example with latest version (0.2.4) and I don't see any problem with that relation. image

sgoodm commented 1 year ago

I think @jacobwhall inadvertently broke this failing example by fixing the OSM feature, haha. One of us will find another feature that fails and update shortly

sgoodm commented 1 year ago

These relations should cause same issue (thanks for finding them @jacobwhall)

https://www.openstreetmap.org/relation/286937 https://www.openstreetmap.org/relation/14395965 https://www.openstreetmap.org/relation/6663864 https://www.openstreetmap.org/relation/2783468 https://www.openstreetmap.org/relation/2783471

jacobwhall commented 1 year ago

What I did to fix @sgoodm's initial example was add "outer" roles to some of the members of the relation. While multipolygon relation members are supposed to have explicit roles, it may be worth our while to try and parse them anyways with osm2geojson, as this error is common on OpenStreetMap. Forgive me if this is already handled as I haven't reviewed the relevant code, but that appears to have been the hangup.

rapkin commented 1 year ago

@sgoodm Are you sure that your example doesn't work? I just tried 286937 and it works fine.

image
rapkin commented 1 year ago

@jacobwhall so do you think it's a good idea to treat ways as 'outer' by default?