-
Notifications
You must be signed in to change notification settings - Fork 368
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: Add Stadia Maps image tile server class #2269
Conversation
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.
Just one minor change request and question out of my ignorance :) Thanks for the ping @greglucas!
lib/cartopy/io/img_tiles.py
Outdated
def __init__(self, apikey, style='alidade_smooth', cache=False): | ||
super().__init__(cache=cache) | ||
self.apikey = apikey | ||
if style in ("alidade_satellite", "stamen_watercolor"): |
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.
Some notes on alidade_satellite
that'd be really easy to miss in our docs...
- We don't actually support this as a full rendered style at the moment (this style ID won't actually work).
- The JPGs that do exist under a slightly different URL are for the raw imagery imagery only and are not a standard 256x256 tile but rather @2x 512 tiles, which are intended to be used in conjuction with a vector style.
The terms on this raw imagery are also a little different per our upstream provider (some details here: https://docs.stadiamaps.com/map-styles/alidade-satellite/), so I TL;DR I would suggest not adding this condition for the satellite style or mentioning it. We do intend to add this as a "normal" raster style in the future with the other layers on top, but it's probably best to just remove this for 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.
Thanks for the info, I removed the satellite option for now.
lib/cartopy/io/img_tiles.py
Outdated
def _image_url(self, tile): | ||
x, y, z = tile | ||
return ("http://tiles.stadiamaps.com/tiles/" | ||
f"{self.style}/{z}/{x}/{y}.{self.extension}" |
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.
Dumb question more than anything as I regrettably haven't used CartoPy myself, but are @2x (aka retina or 512 tiles) useful? If so, we do support those on all styles except for Stamen Watercolor by adding @2x
right before the extension.
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.
👍 It is easy enough to add, so I added an extension parameter as well now and it looks good locally to me.
Thanks @greglucas! This will be a nice enhancement switching things over to Stadia. It looks good to me, except I think the desired_tile_form input should be kept? |
Yeah, I wasn't entirely sure what the |
I don't see it explicitly mentioned but plotting, for instance, stamen_toner_labels without having desired_tile_form gives a warning from |
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 create a new class to interact with the new server.
Thanks for identifying that @lgolston. Rather than trying to special-case the various tile styles, I think it might be easier to just go straight to RGBA for all of Stadia so we can also accommodate potential future styles as well without needing to explicitly handle each style's version. This does add an extra alpha channel to the other images when they aren't explicitly needed I guess, but that doesn't seem like too big of a deal to me to simplify the code here for flexibility... |
Sounds good! That idea has actually been floated by @pelson before as well, in #762 (comment)_ |
The Stamen tiles are now being served by Stadia Maps. Stadia Maps requires an API key and has different naming conventions for their tiles, so create a new class to interact with the new server. Replaces #2266
xref: https://stamen.com/stamen-x-stadia-the-end-of-the-road-for-stamens-legacy-map-tiles/
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.
cc @lgolston @ianthetechie