Closed t-g-williams closed 4 years ago
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)
@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)
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 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 🙈
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.
@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!
Hi -- yes I am still here. However, I am by no means a developer and haven't used this module since ~2 years ago. Please feel free to take what I originally did and modify or improve it in any way you see fit. However, I'm not in a position to be taking a lead on this. Good on you for (maybe) stepping up and doing it!! :)
Tim
On Tue, Sep 3, 2019 at 11:24 AM Thomas Dietrich notifications@github.com wrote:
@t-g-williams https://github.com/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!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mvexel/overpass-api-python-wrapper/pull/76?email_source=notifications&email_token=AFSCJ5HDOTQOPPIH6U6FBRLQHYULLA5CNFSM4DNAQJ2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5XSVQY#issuecomment-527379139, or mute the thread https://github.com/notifications/unsubscribe-auth/AFSCJ5DAAPDU73XN4IC3Q73QHYULLANCNFSM4DNAQJ2A .
-- Tim Williams
PhD Candidate University of Michigan Department of Industrial and Operations Engineering
tgwilliams.com | reckoningrisk.com tgw@umich.edu (+1) 734 353 5577
Code is a bit messy, but it works and it might help others who have had this problem