Skip to content
This repository was archived by the owner on Jun 10, 2025. It is now read-only.

added support for relations #76

Merged
merged 2 commits into from
Dec 10, 2019
Merged

added support for relations #76

merged 2 commits into from
Dec 10, 2019

Conversation

t-g-williams
Copy link
Contributor

Code is a bit messy, but it works and it might help others who have had this problem

@Murthy10
Copy link

Murthy10 commented Oct 4, 2017

I suggest to handle ways with the same starting and ending point as Polygons, for example like

        elif elem_type == "way":  
            points = []  
            for coords in elem["geometry"]:  
                points.append((coords["lon"], coords["lat"]))  
            if points[0] == points[-1]:  
                geometry = geojson.Polygon([points])  
            else:  
                geometry = geojson.LineString(points)  

@t-g-williams
Copy link
Contributor Author

@Murthy10 that's a good idea. I didn't actually change any code for the "way" (just the "relations"), so perhaps you could suggest it as a separate pull request?

Looking at the OSM wiki (http://wiki.openstreetmap.org/wiki/Way), it says "A closed way may be interpreted either as a closed polyline, or an area, or both", and it gives examples of when Ways should be interpreted as areas or closed polylines (they should be polylines for highways and barriers). So perhaps you could modify your code:

if points[0] == points[1] and not any(k in ['highway", "barrier"] for k in elem["tags"].keys()):
    geometry = geojson.Polygon([points])
else:
    geometry = geojson.LineString(points)

@mvexel
Copy link
Owner

mvexel commented Apr 6, 2018

Copying from #48

I think we can go ahead and merge this once we have some test coverage. The code is a little hard to parse, but I think we can work on cleaning it up some after we merge and release.

@t-g-williams can you work on the test coverage?

@mvexel mvexel added this to the 0.7 milestone Apr 10, 2018
@ThomDietrich
Copy link
Contributor

@mvexel this PR is two years old and seems very important for the function this project offers. Any chance we can make some progress here? What is needed to merge? You mentioned test coverage but that shouldn't delay function for two years 🙈

@tbolender
Copy link
Collaborator

Currently, my personal interest in the project is minor. However, you are welcome to participate!

For a merge, the conflict has to be resolved and test covering the parsing of basic structures have to be written.

@ThomDietrich
Copy link
Contributor

@t-g-williams are you still active and willing to push this to successful merge?

All: I'm considering to resolve this issue. Besides test coverage, do you see any other feature or functionality (e.g. multipolygon) that we should keep in mind? Are there unpleasant limitations of the current implementation?

Best!

@t-g-williams
Copy link
Contributor Author

t-g-williams commented Sep 4, 2019 via email

@mvexel mvexel merged commit 51c4ae9 into mvexel:master Dec 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants