-
Notifications
You must be signed in to change notification settings - Fork 366
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
FIX: Change Stamen url over to Stadia Maps #2266
Conversation
The Stamen tiles are now being served by Stadia Maps. Stadia maps requires an API key, and has different naming convention for their tiles, so update both of those things here. https://stamen.com/stamen-x-stadia-the-end-of-the-road-for-stamens-legacy-map-tiles/
'stamen_terrain_lines': {'extension': 'png', 'opaque': False}, | ||
'stamen_toner_background': {'extension': 'png', 'opaque': True}, | ||
'stamen_toner': {'extension': 'png', 'opaque': True}, | ||
'stamen_toner_hybrid': {'extension': 'png', 'opaque': 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.
Toner hybrid is now deprecated according to https://docs.stadiamaps.com/map-styles/stamen-toner/
apikey=None): | ||
if apikey is None: | ||
raise ValueError("An apikey is now required to use Stamen maps. " | ||
"Visit https://maps.stadiamaps.com/ for more details.") |
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.
Dead URL? Suggest instead: https://docs.stadiamaps.com/authentication/
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.
Yes, I can confirm this is the correct URL. maps.stadiamaps.com
is not a valid URL.
style = f"stamen_{style.replace('-', '_')}" | ||
warnings.warn("Stamen maps now require the stamen_ prefix for " | ||
f"the style name, please update to the new style: {style}") | ||
|
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.
This gets to your question about potentially switching the class name over to Stadia, but the name replacement here does not seem ideal since it would prevent using styles now available like Alidade Smooth
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.
So you would be in favor of making a new Stadia
class that has all of their styles?
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.
Yes, that seems preferable to me. Since the user has to get an api key from Stadia, might as well make all the styles available!
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.
Yeah, this is the approach taken by OpenLayers, Leaflet Providers, and more. It's already a breaking change, so changing the name makes sense.
You can find a full list of our styles here: https://docs.stadiamaps.com/themes/. Some notes:
- Stamen styles are (currently) the only ones with multiple variations per style. All others have just one identifier.
- Alidade Satellite is not currently available as pure raster tiles; we expect that to change in the future, but probably doesn't make sense to include that one now.
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.
Hey, thanks for opening this PR @greglucas! I'm one of the cofounders of Stadia Maps, and it looks like we completely missed CartoPy in our scan for open-source projects to open PRs with!
I just added a few comments for completeness in addition to what @lgolston already mentioned.
apikey=None): | ||
if apikey is None: | ||
raise ValueError("An apikey is now required to use Stamen maps. " | ||
"Visit https://maps.stadiamaps.com/ for more details.") |
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.
Yes, I can confirm this is the correct URL. maps.stadiamaps.com
is not a valid URL.
style = f"stamen_{style.replace('-', '_')}" | ||
warnings.warn("Stamen maps now require the stamen_ prefix for " | ||
f"the style name, please update to the new style: {style}") | ||
|
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.
Yeah, this is the approach taken by OpenLayers, Leaflet Providers, and more. It's already a breaking change, so changing the name makes sense.
You can find a full list of our styles here: https://docs.stadiamaps.com/themes/. Some notes:
- Stamen styles are (currently) the only ones with multiple variations per style. All others have just one identifier.
- Alidade Satellite is not currently available as pure raster tiles; we expect that to change in the future, but probably doesn't make sense to include that one now.
Please see the attribution notice at http://maps.stamen.com on how to | ||
attribute this imagery. | ||
For Toner and Terrain: | ||
Map tiles by Stamen Design, under CC BY 4.0. Data by OpenStreetMap, under ODbL. | ||
For Watercolor: | ||
Map tiles by Stamen Design, under CC BY 4.0. Data by OpenStreetMap, under CC BY SA. |
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.
The updated Stamen styles have some updated attribution requirements, which you can find here: https://docs.stadiamaps.com/attribution/. We've also shortened the text for each required attribution, since there are more of them.
Attributions needed for all Stadia Maps styles
- Stadia Maps
- OpenMapTiles
- OpenStreetMap
Additionally, Stamen styles require a credit to Stamen. The page linked above has recommended text.
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.
Thanks for chiming in here! I just pinged you on #2269 which I opened with the new style class for this. I put in the updated attribution text, let me know if you want more in the docstring or I missed something.
The Stamen tiles are now being served by Stadia Maps. Stadia maps requires an API key, and has different naming convention for their tiles, so update both of those things here.
xref: https://stamen.com/stamen-x-stadia-the-end-of-the-road-for-stamens-legacy-map-tiles/
I could also see making a new
Stadia
class for this instead, and deprecatingStamen
? I'm not sure what the best approach is, especially because theStamen
class is not even usable in its current form, so not sure it even needs a deprecation...Apparently we don't have any tests for this... I did make an API key and test locally with the examples and those did seem to work, but depending on the route we go we should probably add a few tests at least.
Rationale
Implications