tilezen / mapbox-vector-tile

Python package for encoding & decoding Mapbox Vector Tiles
MIT License
240 stars 47 forks source link

mapbox-vector-tile doesn't output rfc7946 geojson compliant feature type #107

Open enricofer opened 5 years ago

enricofer commented 5 years ago

in https://github.com/tilezen/mapbox-vector-tile/blob/46629e478935954fc2dcbd8d17d005546b8d31a0/mapbox_vector_tile/decoder.py#L48 mvt decoder assign an invalid geojson feature type that as from geojson specifications should be "Feature", "FeaturesCollection" or "GeometryCollection", not 1,2,3 as from actual implementation. The output geojson isn't actually readable by last GDAL

zerebubuth commented 5 years ago

Yup, sorry about that! The decoder outputs a data structure which is pretty close to the internal structure of an MVT tile, and wouldn't be usable directly as GeoJSON. I think in addition to properly stringifying the enum, we'd also need to do the coordinate conversion from MVT tile-local coordinates back to EPSG:4326.

It's something we'd love to take as a contribution, if you're interested? However, if you're looking for a quick way to convert MVT to GeoJSON, then I'd recommend Mapbox's own vt2geojson, which can output a proper GeoJSON file readable in QGIS for each layer.

enricofer commented 5 years ago

Given that I'm using mapbox-vector-tile in a limited context (reading mapillary coverage in a QGIS plugin) and I don't know and I'm not interested in whole MVT cases implementation, I solve for myself manually setting line 48 of decoder.py forcing to:

"type": "Feature" 

In fact looking to the given link, the internal structure of MVT return always a Features array. I don't know if MVT could even output "GeometryCollection" or "FeaturesCollection". If so the previous fix should suffit.

About coordinate conversion, I'm using a customized decoder.py that equalize MVT tile coordinates to wgs84 using the calculated bounds for the requested tile passed to the optional quantize_bounds parameter:

https://github.com/enricofer/go2mapillary/blob/e58bad68298706848b6fe2fbf5cf8ed41e6317f3/mapillary_coverage.py#L273

You can find the implementation in: https://github.com/enricofer/go2mapillary/blob/master/extlibs/mapbox_vector_tile/decoder.py

here https://github.com/enricofer/go2mapillary/blob/e58bad68298706848b6fe2fbf5cf8ed41e6317f3/extlibs/mapbox_vector_tile/decoder.py#L27

and here: https://github.com/enricofer/go2mapillary/blob/e58bad68298706848b6fe2fbf5cf8ed41e6317f3/extlibs/mapbox_vector_tile/decoder.py#L147

If you found this useful I can submit a PR.

nvkelso commented 5 years ago

Seems useful to wider group of people, so yes on a PR.

But seems like it should be an new option to output rfc7946 geojson than changing the default behavior, which this would be a downpayment for.