-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Plotly map tiles support #4686
Conversation
- Scatter - Curve - Labels - RGB - Shapes - Other Tiles
There was a problem hiding this 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.
Co-authored-by: James A. Bednar <jbednar@users.noreply.github.com>
Co-authored-by: James A. Bednar <jbednar@users.noreply.github.com>
Thanks for taking a look @jbednar!
What I liked about making them static methods is that it avoids the need to import them separately from the Regardless, it's not a major point and I'm happy to move them somewhere else if someone has a suggestion. |
Echoing Jim...this looks great!
Why not both? :-) I would be happy for there to be staticmethods on |
Works for me! Does |
Makes sense to me at least! |
as utility functions
Ok, in 5c0a4a9 I moved the implementations to standalone functions that are called from the |
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! |
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. |
This all looks good to me but I have some questions about projections.
|
holoviews/tests/element/testtiles.py
Outdated
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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!
Thanks for taking a look @philippjfr
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.
I think Mapbox handles the wrap-around internally, but I'll visually confirm this for the various element types this evening.
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 |
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.
GeoViews defines one operation called
That's fine and once the pandas test issues are fixed I'm happy to merge. |
Yeah, that's right. So geoviews would run a lat/lon image through cartopy to distort it into Web-mercator? Very cool!
Awesome, I'll fix the tests and make that GeoViews issue this evening. |
But sadly fairly slow...
Great, thanks! |
A quick check with a long line that goes all the way around the world. 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. |
Tests passing. I think this is ready to go now @philippjfr |
Thanks, this is really exciting! |
Thanks! |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
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
, andSegments
.The
Range*
,Bounds*
, andSelection1d
streams are all supported for traces overlayed on top ofTiles
.As a consequence of this feature set,
datashader
andlink_selections
both work on elements overlayed onTiles
.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
andTiles.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 aTiles
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
datashader
andlink_selections
over vector tile layerRelationship 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