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

Support xyzservices TileProvider objects in hv.Tiles #5057

Closed
martinfleis opened this issue Aug 7, 2021 · 6 comments
Closed

Support xyzservices TileProvider objects in hv.Tiles #5057

martinfleis opened this issue Aug 7, 2021 · 6 comments
Labels
type: enhancement Minor feature or improvement to an existing feature
Milestone

Comments

@martinfleis
Copy link
Contributor

Is your feature request related to a problem? Please describe.

With the @geopandas team, we have built a new package called xyzservices to serve as a unified place for XYZ tile management. The idea behind this is that other packages will depend on that instead of having their own system and list of supported tiles hard-coded. It would be great to have holoviews on board with the idea, either by supporting xyzservices.TileProvider as an input of hv.Tiles or even refactoring tiles.py module to depend directly on xyzservices.

xyzservices itself does not have any dependencies which makes it a non-problematic dependency.

It is worth mentioning that xyzservices stores providers' metadata in JSON under share, accessible for anyone if you have a use case for that.

I am happy to do any changes in xyzservices needed to make it as useful for you as possible if that allows centralising maintenance of providers in one place.

Describe the solution you'd like

  1. support TileProvider as an input of hv.Tiles.
import xyzservices.providers as xyz

hv.Tiles(xyz.CartoDB.Positron)

All the info Tiles require can be fetched from the TileProvider object under the hood.

  1. refactor tile management

You can remove a big chunk of the maintenance burden by refactoring tile management in tiles.py using xyzservices directly. In the first step, you can remove all metadata and simply map your existing classes to TileProvider objects. In the second, you can even dynamically support the whole xyzservices library of providers instead of the pre-defined set you have now.

Describe alternatives you've considered

I know that 2) may sound like a bit ambitious proposal but 1) is pretty straightforward to implement already. So I'd say that having at least the first part would be nice.

Additional context

Support of xyzservices is being implemented across the ecosystem, from geopandas' own packages (new built-in foilum-based plotting and contextily) to leafmap (opengeos/leafmap#91) and ipyleaflet (jupyter-widgets/ipyleaflet#854) (+ hopefully more).

If there's anything we would need to implement in xyzservices to make all this work, I am all ears.

It seems that @philippjfr would welcome that :).

@jlstevens
Copy link
Contributor

jlstevens commented Aug 9, 2021

Thanks @martinfleis for the suggestion!

Interestingly enough, @jbednar @philippjfr and myself recently discovered xyzservices and briefly discussed this suggestion already!

It is good to have an issue opened though and from what I remember, we were thinking that xyzservices could be an optional dependency that could be used instead of replacing the existing tile source definitions. I don't why we couldn't support xyzservices in the constructor as in suggestion 1.

Suggestion 2 would be a bigger effort that I think we should consider (especially as we recently had issues with the wikipedia tile source no longer being public available). While that might need some discussion, I don't think anyone will object to suggestion 1 right now.

If you are willing to make a PR on HoloViews implementing suggestion 1, that would be much appreciated! My only request would be that this is coordinated with a corresponding PR in GeoViews (as we want to maintain a consistent API between the two projects).

@jlstevens jlstevens added this to the v2.0 milestone Aug 9, 2021
@jlstevens jlstevens added the type: enhancement Minor feature or improvement to an existing feature label Aug 9, 2021
@martinfleis
Copy link
Contributor Author

@jlstevens Cool. I'll prepare PRs to implement suggestion 1, mostly mirroring the implementation in folium (python-visualization/folium#1498) which does not even import xyzservices while supporting it.

@jlstevens
Copy link
Contributor

That sounds ideal, thanks!

@xTerma
Copy link

xTerma commented Aug 23, 2021

🈯️🈯️🈯️

@xTerma
Copy link

xTerma commented Aug 23, 2021

@ahuang11
Copy link
Collaborator

I think this can be closed since the HoloViews PR has already been merged.

GeoViews one is added here holoviz/geoviews#654

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Minor feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants