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

ENH: support xyzservices #1498

Merged
merged 4 commits into from Nov 10, 2022
Merged

Conversation

martinfleis
Copy link
Collaborator

Closes #1497

The PR is ready, we just need to wait for xyzservices 2021.08.0 to be released as it currently requires functionality available only on main. That shouldn't take more than a couple of days.

docs/conf.py Outdated Show resolved Hide resolved
tests/test_raster_layers.py Outdated Show resolved Hide resolved
@martinfleis
Copy link
Collaborator Author

Looking at your management of built-in tiles, there's a possibility to simplify all that by relying on xyzservices directly.

In TileLayer we can map those 10 currently built-in on relevant TileProvider objects and get rid of all that complicated machinery you have to manage templates/tiles. We could even map any custom non-URL string to matching TileProvider (as "esriworldtopomap" would be mapped to xyz.Esri.WorldTopoMap, essentially expanding the number of built-in tiles to the whole library we have in xyzservices. That would essentially outsource the maintenance of tile sources to xyzservices.

It would require having xyzservices as a hard dependency though but keep in mind that it itself has no deps.

It is up to you. I can either expand this PR or make the refactor in another one if you'd like to have it.

@martinfleis
Copy link
Collaborator Author

xyzservices 2021.08.0 is now available on PyPI so the tests should now pass.

Copy link
Member

@Conengmo Conengmo left a comment

Choose a reason for hiding this comment

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

Nice integration. Let's merge it when the tests pass.

Additionally, we might want to include an example in our Quickstart example notebook or perhaps a separate notebook.

@Conengmo Conengmo added ready PR is ready for merging and removed waiting for review PR is waiting to be reviewed labels Nov 10, 2022
@martinfleis
Copy link
Collaborator Author

Additionally, we might want to include an example in our Quickstart example notebook or perhaps a separate notebook.

Do you want it to be a part of this PR or a follow-up?

@Conengmo
Copy link
Member

Let's do it as a follow-up, since this one is good to go!

@Conengmo Conengmo merged commit aa6260d into python-visualization:main Nov 10, 2022
@martinfleis martinfleis deleted the xyzservices branch November 10, 2022 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready PR is ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support xyzservices.TileProvider as an input of tiles
3 participants