search

mapbox / vector-tile-spec

Mapbox Vector Tile specification
https://www.mapbox.com/vector-tiles/specification/
Other
900 stars 211 forks source link

Clarify that a feature ID must be an integer #87

Closed mike-marcacci closed 6 years ago

mike-marcacci commented 8 years ago

Since GeoJSON feature IDs are presumably allowed to be any scalar type, and are often strings, this should help clarify that vector tile features must have integer id's if defined.

flippmoke commented 8 years ago

Not suggesting that this change isn't a good idea but currently we do not describe all the expected types as they are clearly defined in the .proto file. However, perhaps we should provide a more proper description of each of the attributes?

https://github.com/mapbox/vector-tile-spec/blob/master/2.1/vector_tile.proto#L32

mike-marcacci commented 8 years ago

Hi @flippmoke - thanks for the reply! That definitely makes sense, and feel free to close this out if you'd like. The only reason I added this here is because it's so common (at least in my circles) for ID fields to be strings that I felt this type restriction deserved a bit of a special note in the docs. In my particular case this isn't a terribly big deal – I can reduce my identifiers to 64 bits and switch between hex/int as I need to – but if I already had a large database of features that used UUIDs, for example, I'd like to know about this when planning rather than half-way into the implementation.

Again, if you feel this would create an expectation for types to exist elsewhere in the docs, I totally could see that and have no problem having this PR closed unmerged :) Just a thoughtful suggestion!

flippmoke commented 8 years ago

@mike-marcacci The writing of the specification was intended for implementors of encoders and decoders specifically and not necessarily the users of vector tiles -- I am not sure we want to change that, but I do think having as much of the specification as possible to be declared in the text of the specification rather then the proto file is a good thing.

Currently we do have a general catch all at the top of section 4 .. https://github.com/mapbox/vector-tile-spec/tree/master/2.1#4-internal-structure

joto commented 6 years ago

Version 3 will allow both integer and string ids and has wording for this in the spec. Closing this PR.