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

Attempt to fix invalid way geometries #105

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mojodna
Copy link
Collaborator

@mojodna mojodna commented Jul 3, 2019

Overview

geom.buffer(0) can often fix invalid geometries, e.g. self-crossing polygons. This increases the number of way geometries produced from certain datasets.

Checklist

  • Add entry to CHANGELOG.md

@mojodna mojodna force-pushed the fix-invalid-way-geometries branch from 2bb7c82 to 36c46e1 Compare July 3, 2019 23:03
@mojodna mojodna added this to the v1.0.0 milestone Jul 3, 2019
@jpolchlo
Copy link
Contributor

jpolchlo commented Jul 9, 2019

This is a potentially costly operation that may not be desired by the user. Should this be triggered with a flag (that defaults to false)? Alternatively, if invalid ways are being discarded (not sure if they are), is the right thing to leave them in the dataframe? Not all operations care about validity, and one can process the geometries like this explicitly, can they not? We could provide a library method in OSM to do what you've proposed.

@mojodna
Copy link
Collaborator Author

mojodna commented Jul 18, 2019

'Tis costly indeed, but it's only attempted on geometries that are found to be invalid.

Prior to this change, invalid way geometries produced null geometries (but retained the rest of the metadata). I think that's wrong either way.

My inclination is make it optional but default to fixing geometries. It feels more useful to be handed a dataframe of geometries that are as valid as possible rather than put the onus of fixing invalid ones on the user. I.e. the expectation should be that things are good to go.

There's no guarantee that way geometries are 1:1 with the OSM element that they're derived from (there might be cases where the invalid geometry is helpful, but I'm struggling to come up with one).

@jpolchlo
Copy link
Contributor

Fair enough.

Tangentially, today I've been encountering empty geometries which are causing any number of headaches. I can't give an example of one because I just want to finish this job and get on with my life, but they are out there. (Encountered these during processing of the most recent planet snapshot in s3://osm-pds/.)

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

Successfully merging this pull request may close these issues.

2 participants