Skip to content

Implement WMTS endpoints - #27

Merged
jdw-creare merged 5 commits into
developfrom
feature/wmts
May 7, 2026
Merged

jdw-creare merged 5 commits into
developfrom
feature/wmts

Conversation

@scranford1

Copy link
Copy Markdown
Contributor

Implement WMTS based on version 1.0.0 of the standard.

Comment thread ogc/edr/test/conftest.py
@scranford1
scranford1 marked this pull request as ready for review May 5, 2026 13:26
@scranford1
scranford1 requested a review from jdw-creare May 5, 2026 13:26

@jdw-creare jdw-creare left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread ogc/settings.py Outdated
# 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment thread ogc/test/test_servers.py
@scranford1

Copy link
Copy Markdown
Contributor Author

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.

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.

@sonarqubecloud

sonarqubecloud Bot commented May 6, 2026

Copy link
Copy Markdown

@scranford1

scranford1 commented May 6, 2026 •

Copy link
Copy Markdown
Contributor Author

@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.

@jdw-creare

Copy link
Copy Markdown
Contributor

This approach looks great! Thanks Sam!!

@jdw-creare
jdw-creare merged commit c04cf25 into develop May 7, 2026
5 checks passed
@scranford1
scranford1 deleted the feature/wmts branch May 8, 2026 19:09
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