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

Why max_geometry_validate_tries parameter setted to 5 #76

Open
engobi opened this issue Nov 8, 2016 · 1 comment
Open

Why max_geometry_validate_tries parameter setted to 5 #76

engobi opened this issue Nov 8, 2016 · 1 comment

Comments

@engobi
Copy link

engobi commented Nov 8, 2016

Hello,

We work actually for adding new geometry validity function on invalid multipolygons that are not yet validate by existing geometry validity function. When working on this issue, we've discovered max_geometry_validate_tries param in class VectorTile (https://github.com/Mappy/mapbox-vector-tile/blob/master/mapbox_vector_tile/encoder.py), this param is setted to 5 that indicate that on the same invalid geometry handle_shape_validity method could be called til 5 time. Why this choice of value (5)? Why this param could not be setted actually when we call mapbox_vector_tile.encode function?

Our forked repository is here : https://github.com/Mappy/mapbox-vector-tile

Thanks

Thierry B.

@zerebubuth
Copy link
Member

Hi!

Why this choice of value (5)?

It used to be possible for the code which made the geometry valid to insert points at non-integer coordinates. This meant that the geometry had to be quantized again, potentially making it invalid again. We found that running through the process a certain number of times could "iterate" towards a valid and quantized geometry. Unfortunately, in some cases the iteration never converged and so we added the max_geometry_validate_tries parameter to escape the loop if convergence didn't seem possible.

The number 5 was chosen by observation to be large enough that most geometries which were going to converge would have, while being low enough that geometries which weren't going to converge didn't take too much time.

Why this param could not be setted actually when we call mapbox_vector_tile.encode function?

I think that was just an oversight. It looks like it would be easy to pass that optional parameter through the encode function here: https://github.com/tilezen/mapbox-vector-tile/blob/master/mapbox_vector_tile/__init__.py#L11-L14

We'd love to get a PR from you with this improvement!

Hope that helps.

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

No branches or pull requests

2 participants