Coordinates conversion #127
Conversation
mapbox_vector_tile/__init__.py
Outdated
| def decode(tile, y_coord_down=False): | ||
| vector_tile = decoder.TileData() | ||
| message = vector_tile.get_message(tile, y_coord_down) | ||
| def decode(tile, y_coord_down=False, crs_from=None, crs_to=None): |
There was a problem hiding this comment.
I think it'd be nice to have this be a little bit more general, and just offer the ability to specify a transform fn: (Point -> Point). This could also mean that we wouldn't need to depend on pyproj, and could just add an example to the readme showing how it could be done.
(Don't mean to take the point -> point part literally, just have it take a (int,int) tuple or 2 parameters)
pyproject.toml
Outdated
| pyclipper = "^1.3.0" | ||
| pyproj = "^3.4.1" |
There was a problem hiding this comment.
if we want to keep the pyproj dependency, it would be nice to relax this to being optional, with a run time check if it's used. Adding pyproj generally sounds heavy to me just to support this scenario. Even if you're on board with the transform fn idea as the specification parameter and we don't technically need pyproj, we might want to offer a convenience wrapper to facilitate the common pyproj usage. (If this doesn't make sense, I can make a pr or write the code out to help explain)
Seems like an extra would work here, right? Something like:
| pyproj = "^3.4.1" | |
| pyproj = { version = "^3.4.1", optional = true } | |
| [tool.poetry.extras] | |
| proj = ["pyproj"] |
Regardless, can we add a note to the readme for this case?
There was a problem hiding this comment.
I will apply this. 👍
mapbox_vector_tile/decoder.py
Outdated
| geometry = self.parse_geometry(feature.geometry, feature.type, layer.extent, y_coord_down) | ||
| new_feature = {"geometry": geometry, "properties": props, "id": feature.id, "type": feature.type} | ||
| geometry = self.parse_geometry(geom=feature.geometry, ftype=feature.type, extent=layer.extent) | ||
| new_feature = {"geometry": geometry, "properties": props, "id": feature.id, "type": "Feature"} |
There was a problem hiding this comment.
can we grow an option that would keep the previous behavior? My guess would be that it's not practically being checked anywhere, but this would be insurance if it were.
mapbox_vector_tile/decoder.py
Outdated
| geometry = self.parse_geometry(feature.geometry, feature.type, layer.extent, y_coord_down) | ||
| new_feature = {"geometry": geometry, "properties": props, "id": feature.id, "type": feature.type} | ||
| geometry = self.parse_geometry(geom=feature.geometry, ftype=feature.type, extent=layer.extent) | ||
| new_feature = {"geometry": geometry, "properties": props, "id": feature.id, "type": "Feature"} |
There was a problem hiding this comment.
tangentially, we might want to also collect a list of the user facing changes and document them in the readme under an upgrading section.
There was a problem hiding this comment.
Of course. I would like first to stabilize the interface before writing the documentation.
|
@rmarianski I will add tests and example in the README after you validate the main idea of |
mapbox_vector_tile/__init__.py
Outdated
| def decode(tile, options=None, default_options=None): | ||
| vector_tile = decoder.TileData(pbf_data=tile, options=options, default_options=default_options) | ||
| message = vector_tile.get_message() | ||
| return message | ||
|
|
||
|
|
||
| def encode( | ||
| layers, | ||
| quantize_bounds=None, | ||
| y_coord_down=False, | ||
| extents=4096, | ||
| on_invalid_geometry=None, | ||
| check_winding_order=True, | ||
| ): | ||
| vector_tile = encoder.VectorTile(extents, on_invalid_geometry, check_winding_order=check_winding_order) | ||
| def encode(layers, options=None, default_options=None): | ||
| vector_tile = encoder.VectorTile(default_options=default_options) |
There was a problem hiding this comment.
nit: can we be more explicit and call it layer_options and default_options? Or per_layer_options?
Also, since we've hoisted the options all under a top level key, what do you think of adding detailed docstrings for the encode/decode functions, and enumerating all options and explaining how they work? I expect that having all the specifics listed out here would go a long way towards making it easy for others to use. Happy to just leave this off for a separate pr though.
There was a problem hiding this comment.
Could you please read the documentation I wrote? I am not a native English speaker, so you may find some mistakes.
There was a problem hiding this comment.
this looks great to me, thanks!
I have implemented in this PR:
pyproj, whose last versions require python3.8, I updated the minimal version of Python@rmarianski This PR needs additional work. I would like to have your opinion on the coordinate conversion.