-
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
Add a cartopy.io.img_tiles.Stamen class #1188
Conversation
@QuLogic - updated as per review. Thanks for the feedback. |
Looking at the warnings in the doc build, it seems like there might be a few other examples that use the |
Oh nice! |
|
||
import cartopy.crs as ccrs | ||
|
||
|
||
class GoogleTiles(object): | ||
class GoogleWTS(six.with_metaclass(ABCMeta, object)): |
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.
I love that this is getting the WTMS
in the name but is it still worth keeping "Google" in there? Why not taking this opportunity to make the class name more generic? I believe GoogleWTS
may throw users, who don't read the docs, off by thinking it only works for Google Tiles.
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.
A fine observation. I agree entirely with the sentiment, but it is really hard to find a name that doesn't include the word Google - this isn't a general WMTS implementation AFAICT - it really does build upon Google mercator and uses (as far as I can tell) a non-standard y indexing scheme.
I suspect there are further abstractions that would be useful (there is a 'Microsoft' Quadtree implementation in here that really only needs a small subset of the GoogleWTS class's behaviour).
In short - I'm 100% in favour of finding a name that represents the scheme that Google implemented (I think it was them at least), but at present don't have a better suggestion.
@QuLogic - merge at will. |
Rationale
Replaces #1013 to generalise the Stamen class of image tiles, thus giving access to all existing and future Stamen styles (following the same URL scheme).
Also adds a more abstract GoogleWTS tiler class to allow more appropriate subclassing.
Implications