search

mapbox / mapboxgl-jupyter

Use Mapbox GL JS to visualize data in a Python Jupyter notebook
MIT License
663 stars 137 forks source link

Support vector tile source for additional viz types #120

Closed akacarlyann closed 5 years ago

akacarlyann commented 6 years ago

Adds support for vector tile source for MapViz, CircleViz, GraduatedCircleViz and HeatmapViz via the VectorMixin class. Adds support for allowing JSON/GeoJSON join-data to be passed as data argument as a filename, URL, or dictionary. Closes #71.

akacarlyann commented 6 years ago

@ryanbaumann Still working on tests and docs for this PR, but it's pretty close :)

akacarlyann commented 6 years ago

@ryanbaumann Can you take a look and see if the tests I've included are sufficient? Still need to update docs, but I should be able to get this done before the weekend :)

ryanbaumann commented 6 years ago

@akacarlyann looks like the external vector tile support for circle, graduatedcircle, and heatmap layers is all based on using the data-join technique. One use case we want to also support is to style data based on properties already in the vector tile itself - is that possible with your new branch here?

For example, if all we wanted to do was replace the geojson sources in this example https://nbviewer.jupyter.org/github/mapbox/mapboxgl-jupyter/blob/master/examples/notebooks/point-viz-types-example.ipynb with vector tile sources. No data-join required from a local data frame, just sourcing the data from an external vector tile source.

akacarlyann commented 6 years ago

Sure, that was working in one of the intermediate steps. I'll make sure it's still working with the idea be that data can be passed in as [] or None. I think the heatmap example still works with an empty list for data maybe...

akacarlyann commented 6 years ago

Hey @ryanbaumann , I've added a flag for disabling vector data join. I'm also working on making sure join data is supported when passed as JSON/GeoJSON, a dictionary, or a filepath, so feel free to take a look.

ryanbaumann commented 6 years ago

Going to try testing this now on various viz types @akacarlyann

akacarlyann commented 5 years ago

@ryanbaumann Have you had a chance to take a look at this? Hoping to spend some time on the project later this week :)

ryanbaumann commented 5 years ago

Awesome @akacarlyann. Unfortunately no, I haven't had much time to check this out. Thank you for the reminder - I'll do it ASAP.

ryanbaumann commented 5 years ago

Largely this looks good @akacarlyann and will be super powerful for adding vector data sources.

One question - could eliminate the need for the disable_data_join=True flag when using a vector tile source? It should be the default behavior that most users will want if they are not making a Choropleth map.

akacarlyann commented 5 years ago

@ryanbaumann Can you explain a little more of what you mean? Under the current vector visualization paradigm, the user has to pass a json data object and the vector tile URL and layer name. But if a user wants to style the layer only using the data within the vector tile source, they must still provide json data for the layer filter applied to the vector data. disable_data_join=True just prevents the empty data param from filtering out all the features in the vector source.

I can refactor this but it will take a little more logic to smartly determine when data should be ignored. Alternately, join data could be added in a separate argument?

ryanbaumann commented 5 years ago

But if a user wants to style the layer only using the data within the vector tile source, they must still provide json data for the layer filter applied to the vector data. disable_data_join=True just prevents the empty data param from filtering out all the features in the vector source.

Understood @akacarlyann. I think this is a reasonable assumption to make then, as long as we document it well, which you have.

Checking out the branch for merge now with the latest changes merged into master.

akacarlyann commented 5 years ago

@ryanbaumann looks like one check is failing now due to updates on master in the last few weeks. I'll look into fixing that, but otherwise, how is this PR looking?

ryanbaumann commented 5 years ago

@akacarlyann looking good with a master update - thank you! Ill approve once passing.

akacarlyann commented 5 years ago

@ryanbaumann ready to go!

ryanbaumann commented 5 years ago

Woho! Thanks for bearing with me on the time to merge - your PR here opens up a huge number of use cases for developers visualizing massive tiled data sources in Jupyter and Python that's too big to fit into the browser memory. Thanks, @akacarlyann!