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

include tiles option for geoshape mark #8767

Open
mattijn opened this issue Mar 9, 2023 · 3 comments · May be fixed by #8885
Open

include tiles option for geoshape mark #8767

mattijn opened this issue Mar 9, 2023 · 3 comments · May be fixed by #8885

Comments

@mattijn
Copy link
Contributor

mattijn commented Mar 9, 2023

With an imperative proof of concept in #5758 (comment) for tile support within Vega-Lite. I'm proposing something as the following for declarative tiles support:

"mark": {"type": "geoshape", "tiles": true} 

Where true defaults to "tiles": {"url": "https://tile.opentopomap.org/", "attribution": "(C) OpenStreetMap contributors"}

similar to "mark": {"type": "line", "points": true}.

This can also be extended with providers that require access-tokens or API keys.

The imperative approach of the poc above can then be included within the extended vega-lite specification.

@domoritz
Copy link
Member

domoritz commented Mar 9, 2023

This is great. I think we could add support for this in a macro.

I don't know how official we want to make this feature as Vega support is somewhat cumbersome right now.

@binste
Copy link

binste commented May 1, 2023

I find it very exciting to see tiles in a VL chart, this is exactly what I was missing in terms of geo support. Thank you @mattijn for figuring out how it works in the PoC!!

I'm a novice at javascript and don't yet fully understand the PoC specs but I think with some guidance I could give it a go and try to implement a macro for this or at least lay a good foundation for someone else to finish it. @mattijn what do you think? Are you already working on this and can I help or ok if I start?

My current understanding is that for a line mark the point property is expanded in:

  • in compile.ts normalize is called → normalizeGenericSpecCoreNormalizer.map -> over super.map it goes to CoreNormalizer.mapUnitfor (const unitNormalizer of this.nonFacetUnitNormalizers) {PathOverlayNormalizer in nonFacetUnitNormalizers.
  • In PathOverlayNormalizer, hasMatchingType checks if this classes run method should be called from the normalizer. The spec is then expanded in the run method

Based on this, I'd assume that it makes sense to expand hasMatchingType to check for the existence of a truthy value for tiles for the geoshape mark and then expand the spec in the run method into the layered spec of the PoC.

@mattijn
Copy link
Contributor Author

mattijn commented May 2, 2023

Hi @binste! Please go ahead with trying to make this work. I did not start working on this.
More than happy to have a look as well if you get stuck somewhere.

@binste binste linked a pull request May 6, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Geospatial visualization
Development

Successfully merging a pull request may close this issue.

3 participants