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

Plotly map tiles support #4686

Merged
merged 18 commits into from Nov 18, 2020
Merged

Plotly map tiles support #4686

merged 18 commits into from Nov 18, 2020

Conversation

jonmmease
Copy link
Collaborator

@jonmmease jonmmease commented Nov 15, 2020

Overviews

This PR enhances the plotly backend with full support for the holoviews.Tiles element for displaying maps.

cc @philippjfr @jlstevens @jbednar

Features

  • holoviews.Tiles element that can display raster maps from a tile server (like the Bokeh backend) or vector tiles maps from the Mapbox service. Vector tiles from mapbox require a free account with mapbox.com and a user token, but no account or token is required otherwise.

  • Many elements can be overlayed on top of Tiles including: Scatter, Points, Curve, Labels, RGB, Path, Rectangles, Bounds, and Segments.

  • The Range*, Bounds*, and Selection1d streams are all supported for traces overlayed on top of Tiles.

  • As a consequence of this feature set, datashader and link_selections both work on elements overlayed on Tiles.

  • A new pair of static methods were added to Tiles class for converting between lat/lon coordinates and pseudo-Mercator coordinates: Tiles.lon_lat_to_easting_northing and Tiles.easting_northing_to_lon_lat

  • All of this works with the new Dash support.

  • I added a simple Tiles documentation page that matches the Bokeh version, but includes a small section on the vector tiles support. I plan to add a few examples to the Dash HoloViews docs PR once this is released.

Challenges

For posterity, here are a few of the challenges I ran into, all a consequence of how plotly.js relies on Mapbox for mapping support:

  • While Mapbox displays maps in the same pseudo-Mercator coordinate system as Bokeh, accepts and returns coordinate in the longitude, latitude coordinate system. For consistency with the current paradigm, the external interface works with pseudo-Mercator and conversions are done internally in all interactions with plotly.js.

  • Plotly has a separate set of traces for display on top of mapbox layers. So the trace used to draw a line on a map is not the same trace as is used to draw a regular 2D line. Same for lines, text, images, etc. To make this work with the HoloViews paradigm, many of the plotly plot classes were updated to be able to render themselves either to a 2D Cartesian coordinates, or on top of a mapbox map layer. This geo mode is activated automatically whenever a geo-supported element is overlayed with a Tiles element. An error is raised on render if an element without geo support is overlayed with a Tiles element.

  • Similarly to above, the zoom/pan/selection events emitted by Plotly.js are different for mapbox subplots than regular 2D Cartesian subplots, and they operate in lat/lon coordinates. So the Plotly callback classes were all updated to handle the mapbox case as well as the Cartesian subplot case.

In the end, I think it all worked out pretty well and the user experience shouldn't be any more complicated than when using the Bokeh backend.

Screenshots

All built-in raster tile maps

Screenshot_20201115_175756

datashader and link_selections over vector tile layer

plotly_holoviews_map_datashader_link_selections

Relationship to other PRs

For testing purposes, this PR contains the changes in #4683 (comment). I'll merge master to fix the conflicts once that one is merged.

I fixed a bug in plotly.js that caused RGB image layers to be pushed behind raster layers. plotly/plotly.js#5269. Until this fix is released, datashader can only really be used on top of the mapbox vector tile layers. After the fix is released in plotly.js, it will also work properly on top of raster tile layers.

TODO

  • Python 2.7 compatibility
  • Visually inspect geo elements that cross the date boundary to confirm whether mapbox handles the wraparound internally.

@jonmmease jonmmease added this to the v1.14.0 milestone Nov 15, 2020
Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I can't see how it could be cleaner, given the impedance mismatch between how Plotly and Bokeh handle projections, and Plotly's restrictions on what can be overlaid on maps.

The web mercator conversion functions look good, but to me it seems like they ought to be bare functions, not static class methods, so that it's easier for people to call them individually. For instance, in the plotly/Tiles.ipynb file, seems like we should suggest that people use those functions to convert their data first.

examples/reference/elements/plotly/Tiles.ipynb Outdated Show resolved Hide resolved
examples/reference/elements/plotly/Tiles.ipynb Outdated Show resolved Hide resolved
holoviews/plotting/plotly/dash.py Outdated Show resolved Hide resolved
holoviews/plotting/plotly/images.py Outdated Show resolved Hide resolved
holoviews/plotting/plotly/tiles.py Outdated Show resolved Hide resolved
holoviews/tests/plotting/plotly/testcallbacks.py Outdated Show resolved Hide resolved
jbednar and others added 5 commits November 15, 2020 21:15
Co-authored-by: James A. Bednar <jbednar@users.noreply.github.com>
Co-authored-by: James A. Bednar <jbednar@users.noreply.github.com>
@jonmmease
Copy link
Collaborator Author

Thanks for taking a look @jbednar!

The web mercator conversion functions look good, but to me it seems like they ought to be bare functions, not static class methods, so that it's easier for people to call them individually.

What I liked about making them static methods is that it avoids the need to import them separately from the Tiles element. In your view, why would bare functions be easier to call individually?

Regardless, it's not a major point and I'm happy to move them somewhere else if someone has a suggestion.

@jlstevens
Copy link
Contributor

jlstevens commented Nov 16, 2020

Echoing Jim...this looks great!

What I liked about making them static methods is that it avoids the need to import them separately from the Tiles element. In your view, why would bare functions be easier to call individually?

Why not both? :-)

I would be happy for there to be staticmethods on Tiles (where they clearly make sense) that import and use functions supplied as utilities (bare functions). That way both use cases are supported and HoloViews would allow you to do these transforms without necessarily mentioning the Tiles element when it isn't relevant.

@jonmmease
Copy link
Collaborator Author

Why not both? :-)

Works for me! Does holoviews.util.transform.(lon_lat_to_easting_northing|easting_northing_to_lon_lat) make sense?

@jlstevens
Copy link
Contributor

Makes sense to me at least!

@jonmmease
Copy link
Collaborator Author

Ok, in 5c0a4a9 I moved the implementations to standalone functions that are called from the Tiles static methods.

@jbednar
Copy link
Member

jbednar commented Nov 16, 2020

In your view, why would bare functions be easier to call individually?

I guess easier isn't the right word; more like "conceptually cleaner". I.e., it feels dirty to call a static method on a class that I'm not using, making me feel like it's somehow tied to that class in particular, and shouldn't be used outside that context. Those functions are fully general, and now that they are available as bare functions, people can feel comfortable using them anywhere they are useful. Thanks!

@jonmmease
Copy link
Collaborator Author

I merge in master, and now the only changes in the PR that are outside of the plotly plotting directory are the addition of the lat/lon coordinate conversion functions.

I don't see anything in the failing tests that look related to this PR, but let me know if there's anything I can help out with there.

@philippjfr
Copy link
Member

This all looks good to me but I have some questions about projections.

  1. How are image projections handled? Does it just project the bounding box? If so won't this cause various distortions?
  2. What happens for objects that cross the date boundary? Is that handled at all or will this cause weird issues wrapping around the entire globe?
  3. How do you imagine extending this work in GeoViews by building on cartopy? Can/should we make the projection code here pluggable so we can simply override it in GeoViews?

self.check_array_type_preserved(
pd.Series, pd.Series,
lambda a, b: pd.testing.assert_series_equal(
a, b, atol=0.01, check_exact=False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

atol does not exist for the py2 pandas version it's getting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry you still have to worry about py2 here!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, thanks for the pointer!

@jonmmease
Copy link
Collaborator Author

jonmmease commented Nov 17, 2020

Thanks for taking a look @philippjfr

This all looks good to me but I have some questions about projections.

How are image projections handled? Does it just project the bounding box? If so won't this cause various distortions?

Mapbox is a little odd here in my opinion. The corners of the image are defined in lat/lon, but Mapbox doesn't apply any distortion to the image when laying it out on the map (i.e. the image pixels don't go through the web-mercator projection). So the image needs to be generated in web-mercator coordinates. So the end result is that it works just like Bokeh, but I have to convert the corners to lat-lon. Let me know if that didn't make sense.

What happens for objects that cross the date boundary? Is that handled at all or will this cause weird issues wrapping around the entire globe?

I think Mapbox handles the wrap-around internally, but I'll visually confirm this for the various element types this evening.

How do you imagine extending this work in GeoViews by building on cartopy? Can/should we make the projection code here pluggable so we can simply override it in GeoViews?

To be honest, I haven't looked much into how GeoViews handles this. What I can say is that Mapbox can't display anything besides Web-mercator. I am interested in working through what from GeoViews would apply here. But is it all right if, for now, I open an issue in GeoViews and we push this off after this HoloViews release?

Edit: Although, plotly does have non-mapbox based map projections using the scattergeo and choropleth traces: https://plotly.com/python/map-configuration/. These look like they might be similar to what Bokeh does in https://geoviews.org/user_guide/Projections.html.

@philippjfr
Copy link
Member

philippjfr commented Nov 17, 2020

So the end result is that it works just like Bokeh, but I have to convert the corners to lat-lon. Let me know if that didn't make sense.

It does make sense but I guess it does introduce distortion if you provide a lat/lon image which is why a proper projection with cartopy is still preferable.

To be honest, I haven't looked much into how GeoViews handles this.

GeoViews defines one operation called gv.project which handles all projecting and dispatches to the correct cartopy routines depending on the type of data, e.g. points, vectors, images, quadmeshes, geometries are all handled separately.

I am interested in working through what from GeoViews would apply here. But is it all right if, for now, I open an issue in GeoViews and we push this off after this HoloViews release?

That's fine and once the pandas test issues are fixed I'm happy to merge.

@jonmmease
Copy link
Collaborator Author

it does introduce distortion if you provide a lat/lon image which is why a proper projection with cartopy is still preferable.

Yeah, that's right. So geoviews would run a lat/lon image through cartopy to distort it into Web-mercator? Very cool!

That's fine and once the pandas test issues are fixed I'm happy to merge.

Awesome, I'll fix the tests and make that GeoViews issue this evening.

@philippjfr
Copy link
Member

Yeah, that's right. So geoviews would run a lat/lon image through cartopy to distort it into Web-mercator? Very cool!

But sadly fairly slow...

Awesome, I'll fix the tests and make that GeoViews issue this evening.

Great, thanks!

@jonmmease
Copy link
Collaborator Author

What happens for objects that cross the date boundary? Is that handled at all or will this cause weird issues wrapping around the entire globe?

A quick check with a long line that goes all the way around the world.

newplot

The Mapbox and easting/northing to lon/lat conversions do the right thing. And mapbox automatically repeats the line as you pan around the world. So I think that's working fine.

@jonmmease
Copy link
Collaborator Author

Tests passing. I think this is ready to go now @philippjfr

@philippjfr philippjfr merged commit b1e606d into master Nov 18, 2020
@philippjfr
Copy link
Member

Thanks, this is really exciting!

@philippjfr philippjfr deleted the plotly_tiles branch November 18, 2020 11:29
@jonmmease
Copy link
Collaborator Author

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

4 participants