tilezen/mapbox-vector-tile

Why max_geometry_validate_tries parameter setted to 5

engobi opened this issue · 1 comments

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.

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.