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

Thoughts on API #8

Open
binste opened this issue May 21, 2023 · 1 comment
Open

Thoughts on API #8

binste opened this issue May 21, 2023 · 1 comment

Comments

@binste
Copy link
Collaborator

binste commented May 21, 2023

  • Does it make sense to more directly integrate with Altair so a user could do mark_geoshape(tile=True) as proposed in Add support for tiles to geoshape mark vega-lite#8885? Once it's implemented in Vega-Lite, I think this is a great syntax but for this package I'm not sure as it would then change the type of the Altair chart to a layered chart. As a user I'd find this transition from alt.Chart -> alt.LayerChart unexpected. Also, this way it's clearer that the functionality comes from a separate package.
  • Are add_tiles and create_tiles_chart good names? add_tiles is inspired from add_basemap in contextily
  • Is it user friendly enough to pass an alt.Projection object to create_tiles_chart or better to expand the arguments, i.e. create_tiles_chart(scale=..., translate=...)?
  • Instead of providing add_tiles and add_attribution functions, we could rely solely on create_..._chart functions and then the returned charts can be layered by the user themselves. For attribution, this would work and might simplify it. For add_tiles, we would always need to create the empty geoshape chart which is created in create_..._chart as else it is not a standalone working chart.
    • Way around this could be to introduce a new class TilesChart which only adds the geoshape layer when to_dict is called and if it is added to another chart it would loose that behavior (overwrite __add__).
    • TilesChart could also help with always keeping the attribution layer on top. There could be an escape hatch to convert it to a normal Altair class.
@mattijn
Copy link

mattijn commented May 21, 2023

Another thought: the create_tiles_chart almost feels like a mark_tiles, with specific mark properties like zoom, but without encoding channels.
I like the concept of TileChart too.

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

No branches or pull requests

2 participants