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

Allow vectortile.Layer to pass a valid 'id' to internal.vector_tile.Tile.Layer #2884

Closed
kevinmhinson opened this issue Mar 22, 2019 · 2 comments · Fixed by #3005
Closed

Allow vectortile.Layer to pass a valid 'id' to internal.vector_tile.Tile.Layer #2884

kevinmhinson opened this issue Mar 22, 2019 · 2 comments · Fixed by #3005

Comments

@kevinmhinson
Copy link

Hi! I'm using GeoTrellis and VectorPipe to create MapBox vector tiles. The problem I have is... 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.

To solve this for my purposes, I forked and compiled locally with a Map[String,Value] => Option[Long] function hard coded in Layer.unfeature() to create an 'id' from 'data' to pass to vt.Tile.Feature. This worked, and the output .PBFs had the expected 'id' in them.

I'm trying to think through how else users might want to create an 'id'... maybe a random Long would be sufficient for some, but that could still be coded into a 'data' => 'id' function.

I believe this would be a common pattern, to want an 'id' and to be able to create from the metadata. It looks like the VectorPipe API has completely changed since two days ago, but it used to be in Collate.generically() where a vectortile.StrictLayer is constructed. If there was a way to pass in a Map[String,Value] => Option[Long] function to Layer/StrictLayer/LazyLayer for Layer to use in .unfeature() to convert 'data' to valid 'id' instead of hard-coding as None... I think that would be a common use case.

@jpolchlo
Copy link
Contributor

Looks like you're absolutely right. The GeoJSON spec indeed does include an optional id property. For reference, https://tools.ietf.org/html/rfc7946#page-11 shows that this identifier should be placed into a separate top-level field of the feature. Given that, it's not clear that the right solution is simply to allow properties to be mined for this data, since that may imply that the id field will be redundantly serialized to both the properties and id field. We may want to include a proper id field in Feature[G, D] itself and let that change propagate into vectortile naturally.

Any thoughts, @echeipesh, @pomadchin ?

@echeipesh
Copy link
Contributor

echeipesh commented Mar 29, 2019

A breadcrumb for where VectorTile feature are being created:

private def unfeature(
keys: Map[String, Int],
values: Map[Value, Int],
geomType: vt.Tile.GeomType,
cmds: Seq[Command],
data: Map[String, Value]
): vt.Tile.Feature = {
val tags = data.toSeq.foldRight(List.empty[Int]) { case (pair, acc) =>
/* These `Option.get` _should_ never fail */
keys.get(pair._1).get :: values.get(pair._2).get :: acc
}
vt.Tile.Feature(None, tags, Some(geomType), Command.uncommands(cmds))
}

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

Successfully merging a pull request may close this issue.

3 participants