geotrellis / vectorpipe

Convert Vector data to VectorTiles with GeoTrellis.
https://geotrellis.github.io/vectorpipe/
Other
74 stars 22 forks source link

Write top level 'id' to output Features #65

Open kevinmhinson opened 5 years ago

kevinmhinson commented 5 years ago

Hi! I need to be able to write a top level 'id' in each Feature (not as part of 'properties')... and in vectortile.Layer.unfeature(), the arg id: Option[Long] passed to vectortile.internal.vector_tile.Tile.Feature is hard-coded in .unfeature() as None. I've opened geotrellis issue #2884 to try and expose the ability to create Layers with a Map[String,Value] => Option[Long] function to convert 'data' => 'id' to pass a valid id along to vt.Tile.Feature().

For that to be useful, vectorpipe.Collate.generically() would have to also accept the same arg and pass along when constructing a vectortile.StrictLayer.

jpolchlo commented 5 years ago

Hi, @kevinmhinson. Thanks for lodging this issue.

As you seem to be aware, VP is currently undergoing a bit of a redesign, and the generation of vectortiles is ripe for improvement since the GenerateVT module was never meant to see the light of day (at least not in wide release as part of a larger library).

As it happens, I pushed the big, green button on the first round of improvements to this library this morning, now available under the v1.0.0-RC1 tag. This replaces ProcessOSM with a slimmed down, DataFrame-based API in OSM. The new vectortile generation code will follow along with this general philosophy, and I'll keep in mind the feature that you're requesting here as the work proceeds.

(Reading between the lines, that does unfortunately imply that we're very unlikely to put any further work into the old interface, and if you need this desperately, it'll unfortunately fall on you to do the leg work yourself—which we would be happy to include in a bugfix release of the 0.3 line if you feel like making a PR.)

On the upside, the changes to vector tile generation are already in progress, and I hope to have something for public consumption on a weeks-not-months timeline (barring any exigencies that take me away from the project).

kevinmhinson commented 5 years ago

Hi @jpolchlo . Awesome news! I appreciate this feedback, and the consideration of this request. The work you guys do is awesome, and really helpful! Thank you!

I'm completely on-board with however you want to implement. I just opened the issue here because ultimately if GT is able to write top-level id's, it will help to have the functionality exposed through VP too. I'm agnostic about how, just wanted to come in with a suggestion instead of a problem.

Re the old interface... I'm in good shape now. I have a locally compiled GT v2.1 with a hard coded solution in Layer, and it plays nicely so far with VP 0.3 as-is. I'll be happy to get back on a standard release though if these issues gain traction.

Thanks again!