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

Add support for complex properties from MVT formats #581

Closed
nvkelso opened this issue Aug 15, 2017 · 6 comments
Closed

Add support for complex properties from MVT formats #581

nvkelso opened this issue Aug 15, 2017 · 6 comments

Comments

@nvkelso
Copy link
Member

nvkelso commented Aug 15, 2017

Tangram JS sister issue to tangrams/tangram-es#1618. Related to the root cause of tangrams/tangram-es#1616.

While GeoJSON and TopoJSON support complex property values (nested properties, lists, etc) those are currently dropped in Tilezen MVT format on the server. We plan to add support for stringifying those property values instead in tilezen/mapbox-vector-tile#94.

This Tangram JS issue covers turning the stringified values back into regular complex property as the tile is parsed.

@matteblair
Copy link
Member

I have concerns about encoding JSON values into MVT string properties. Mainly, the question I have is: How does the engine know which string values should be interpreted as JSON? Possible answers:

  • Try to parse every string value as JSON: This is wasteful because the vast majority of string values are not intended to be JSON, and we would also get false positives for string values that can be validly interpreted as JSON (true, null, etc.).
  • Use a whitelist of property names to parse as JSON: This doesn't align with the data-agnostic spirit of Tangram, plus we'd have to change/version this list if the property names ever changed.
  • Lazily parse properties as JSON if members are accessed: For instance if a JS function accessed [0] or .a of a property, we could parse the string value as JSON in our proxy object before returning a value - though this is getting complicated and it still wouldn't behave exactly the same as Tangram JS would.

@matteblair
Copy link
Member

There are other conceptual solutions to this as well, such as interpreting all feature properties from all formats as JSON values, but that involves more dramatic refactoring.

@bcamper
Copy link
Member

bcamper commented Aug 15, 2017

I agree that knowing which fields to apply extra parsing to is problematic. One thought I had is a variation of your options above, which is that these fields would have to be specified as part of the Tangram data source definition in sources. This requires an extra "mini schema" definition step, but stays agnostic.

@matteblair
Copy link
Member

Ah oops, I meant to post my replies to tangrams/tangram-es#1618 - lots of similar issues in my tabs haha. I suppose the concern still mostly applies here as well.

@bcamper
Copy link
Member

bcamper commented Dec 4, 2019

Support for these properties was added to Tangram JS in https://github.com/tangrams/tangram/releases/tag/v0.19.0. The user must whitelist the MVT properties to be parsed as JSON, or opt into all being parsed and incur a performance hit. No parsing is done by default.

@bcamper bcamper closed this as completed Dec 4, 2019
@nvkelso
Copy link
Member Author

nvkelso commented Dec 4, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants