Skip to content

Remove NASAGIBS Blue Marble example #1918

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

Merged

Conversation

Conengmo
Copy link
Member

@Conengmo Conengmo commented Apr 4, 2024

#1882 was very helpful in showing that the currently listed example tileset doesn't work anymore. But the proposed replacement is not suitable unfortunately. Simply solve this issue by removing the example. If users want more examples, they can visit the site we mention: http://leaflet-extras.github.io/leaflet-providers/preview/

@Conengmo Conengmo requested a review from martinfleis April 4, 2024 17:05
folium/folium.py Outdated
Comment on lines 100 to 99
``http://leaflet-extras.github.io/leaflet-providers/preview/``.
http://leaflet-extras.github.io/leaflet-providers/preview/.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shall we maybe point to xyzservices instead? Either to the docs (the list is here) or with the code snippet

import xyzservices.providers as xyz

xyz

Now that we depend on it, it will be more straightforward to just pass the name of the tiles rather than figuring out on leaflet-providers which URL shall be passed where and how the attribution should look like.

Copy link
Member Author

Choose a reason for hiding this comment

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

it will be more straightforward to just pass the name of the tiles rather than figuring out on leaflet-providers which URL shall be passed where and how the attribution should look like.

Totally agree! I think we can still keep it in the docstring of TileLayer, since it is a more advanced feature we still support, but put it lower in the order.

Shall we maybe point to xyzservices instead?

Since then xyzservices docs point to that leaflet providers preview, I'd rather link to it directly. But how about we link to both? I'll make a change.

Copy link
Member Author

Choose a reason for hiding this comment

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

@martinfleis I streamlined the docstring a bit more, does this look good to you? I hope it's more clear now that: users should mainly provide provider names, and that the xyzservices package is used underneath. Underneath, if they want to do more advanced stuff we still list a link to xyzservices.TileProvider, as well as how to use a custom url.

Copy link
Collaborator

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

Looks good!

@Conengmo Conengmo merged commit aaa3212 into python-visualization:main May 6, 2024
@Conengmo Conengmo deleted the remove-nasagibs-blue-marble branch May 6, 2024 14:31
@Conengmo
Copy link
Member Author

Conengmo commented May 6, 2024

Thanks Martin!

hansthen pushed a commit to hansthen/folium that referenced this pull request Jun 2, 2024
* Remove NASAGIBS Blue Marble example

* update docstrings
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.

2 participants