Implement WMTS endpoints - #27
Conversation
jdw-creare
left a comment
There was a problem hiding this comment.
There's a comment I wanted to leave on ogc/wmts/wmts_routes_1_0_0.py:143, but there's some github bug stopping me, and it's less about that line as much as the structure of the file anyway, so I'll put it here:
I'd like to steer away from using Any in type hints wherever we can. In this particular situation, it feels like we can avoid wmts_request: Any. The use of Any makes it much harder to navigate around the call tree in VS Code, and I would also like to have full advantage of the pre-runtime type checking.
I think i see a lot of logic from prior OGC protocols (namely WMS and WCS) that landed in here, and I think that logic may have been based on questionable premises to begin with, and i think we should either avoid copying it, or lean fully into the abstraction that it's meant to support.
When I read this get_tile method, and its callers, my first impression was that this was a form of dependency injection. I suspect this is why the old WCS and WMS code has it, and it's an abstraction I can get behind. It's like there's a desire to support several versions of each protocol, by passing in the code implementing the specific version. Like when it's called on line 80, we inject the dependency on the specific 1.0.0 version. This is a really solid and useful design pattern! So upon my first reading, I was thinking about ways to avoid the use of Any in the type hinting - and there are definitely ways to do this, it just would require some abstract base class that would represent the essence of WMTS common among all versions.
But this abstraction is like 80-90% of the way there: the file itself has 1_0_0 in the name, and on line 129 we have a hardcoded reference to version 1.0.0. In order to add support for a future version of WMTS, we'd still need to overhaul this file, and update its callers in a substantive way. handle_kv could be lifted out of this file and placed in some version-agnostic file. The class name WmtsRoutes is already version-agnostic. Maybe this wants to become the abstract base class?
Do all WMTS requests specify a version? One structure that would fully lean into this abstraction might look something like this:
# File wmts_routes.py
class WmtsRoutes(ABC):
# Totally version-agnostic, though unfortunately you do have to assume some verbs like GetCoverage will persist into future versions
@staticmethod
def handle_kv(kwargs):
if version==1.0.0:
wmts_routes = WmtsRoutes1_0_0()
else:
# Do absolutely nothing without a version
raise ....
return wmts_routes.handle_kv(kwargs)
# Abstract methods like get_coverage_from_id(), get_tile()
# File wmts_routes_1_0_0.py
class WmtsRoutes(WmtsRoutes)
def handle_kv():
....
def get_capabilities(self, args: Dict[str, Any]) -> str:
# Then you can do version-specific lines like this one
get_capabilities = wmts_request_1_0_0.GetCapabilities()Another direction, which I would also be 100% in support of, would be to ditch the dependency injection abstraction entirely. We have no requirement to support any other version, so it's totally valid to hardcode version 1.0.0 everywhere, and keep that check in place that raises an exception for other versions. This would make type checking work again, since we could point to the specific implementation classes. If we need to support another version in the future, then that requirement will come with the information that we need to make fully informed decisions about an abstraction that's common between the two of them.
| # WMTS tiling parameters | ||
| WMTS_TILE_SIZE = 256 # pixels | ||
| WMTS_PIXEL_SIZE_METERS = 0.00028 # meters/pixel screen equivalent | ||
| WMTS_INITIAL_RESOLUTION = 40075016.686 / WMTS_TILE_SIZE # meters/pixel for the crs bounds (global) |
There was a problem hiding this comment.
This 40075016.686 number feels significant - is that, like Earth's diameter at the equator, or something specified in the standard, or something like that? Feels worthy of a comment or being its own constant.
There was a problem hiding this comment.
Yes, magic number on my end. Circumference of the earth in meters (my comment could have been better). I will make it its own constant.
Valid points. I will adjust this so that the WMTS is actually abstracted properly. This arose mostly from me copying logic from the core.py file so that WMTS would match WCS and WMS (with the addition of the route class). Looking at the core.py file I am realizing we also hardcode version in there as well. I will plan on adjusting it for WMTS and leave the current WMS/WCS logic alone. |
|
|
@jdw-creare After further consideration, I decided to keep a single routes file and move the versioning logic into each function (get_capabilities and get_tile). I realized that WmtsRoutes needs to fallback on a default version for getCapabilities but not for getTile which means that the base WMTS Routes implementation would still need to check requests types before calling a "versioned" router. If additional version support is needed in the future we can instead abstract GetCapabilities, Capabilities, and GetTile. Let me know if this approach works for you. |
|
This approach looks great! Thanks Sam!! |



Implement WMTS based on version 1.0.0 of the standard.