Use of custom tile routes

[RemiSahl]

Hello!

I am trying to add a custom /tile routes on the TilerFactory in order to match my API specifications.
In order to this, I followed the methodology documented here: https://devseed.com/titiler/advanced/customization/#add-a-mosaicjson-creation-endpoint. I created a child class of the TilerFactory which exposes my custom route. I also implemented a custom path_dependency which allows src_path to be resolved using my custom parameters (here: project_id, layer_id, and time).

However, I did not find a way to bind my custom route to the /tiles function defined in the TilerFactory class. Therefore, I had to duplicate the code responsible for fetching a tile in my custom route, and this is potentially a pain to maintain in the future, should the /tile fetch logic changes. Can you think of another way to do this while avoiding code-duplication ?

I initially tried to call the /tile route, but it is apparently not possible: fastapi/fastapi#874.
As advised in the issue, one way to fix this would be to move the code responsible for fetching the tiles using rasterio in a separate module: " the easiest way to accomplish this might be to separate the shared business logic into a reusable function."

This is what the code of my factory looks like for now:

@dataclass
class CustomTilerFactory(TilerFactory):
    """Tiler Factory."""

    def register_routes(self):
        # Load TilerFactory routes
        super().register_routes()

        # Load additional custom routes
        self.customTileRoute()

    def customTileRoute(self):
        @self.router.get(
            r'/wmts/{project_id}/{layer_id}/{time}/{TileMatrixSetId}/{z}/{x}/{y}.{format}',
            description='Get Tile routes for wmts queries', tags=['tile', 'wmts'],
            **img_endpoint_params
        )
        def customTileRoute(
            z: int = Path(..., ge=0, le=30, description="TMS tiles's zoom level"),
            x: int = Path(..., description="TMS tiles's column"),
            y: int = Path(..., description="TMS tiles's row"),
            tms: TileMatrixSet = Depends(self.tms_dependency),
            scale: int = Query(
                1, gt=0, lt=4, description="Tile size scale. 1=256x256, 2=512x512..."
            ),
            format: ImageType = Query(
                None, description="Output image type. Default is auto."
            ),
            src_path=Depends(self.path_dependency),
            layer_params=Depends(self.layer_dependency),
            dataset_params=Depends(self.dataset_dependency),
            postprocess_params=Depends(self.process_dependency),
            colormap=Depends(self.colormap_dependency),
            render_params=Depends(self.render_dependency),
            tile_buffer: Optional[float] = Query(
                None,
                gt=0,
                alias="buffer",
                title="Tile buffer.",
                description="Buffer on each side of the given tile. It must be a multiple of `0.5`. Output **tilesize** will be expanded to `tilesize + 2 * tile_buffer` (e.g 0.5 = 257x257, 1.0 = 258x258).",
            ),
            reader_params=Depends(self.reader_dependency),
            env=Depends(self.environment_dependency),
        ):
            """Create map tile from a dataset."""
            timings = []
            headers: Dict[str, str] = {}

            tilesize = scale * 256

            with Timer() as t:
                with rasterio.Env(**env):
                    with self.reader(src_path, tms=tms, **reader_params) as src_dst:
                        data = src_dst.tile(
                            x,
                            y,
                            z,
                            tilesize=tilesize,

PS: Thanks for developing this library, it's very cool!

[vincentsarago]

Hey @RemiSahl thanks for opening the discussion.

Sadly I don't think there is a better way. I've opened an issue #457 to start thinking about it but still can't find a good solution

[RemiSahl]

Hello @vincentsarago and thank you for answering my question.

If I may, I think moving the rasterio logic outside of the fastApi routes declaration would allow me to import the required piece of code separately and fix my code duplication issue. Moreover, moving the rasterio manipulation logic outside of the Factory code would also open the door for different implementations of the geospatial data manipulation (using gdal python bindings instead of rasterio for example).

[vincentsarago]

@RemiSahl I'm open to suggestion. I think stac-fastapi do use by abstracting using Client https://github.com/stac-utils/stac-fastapi/blob/06f610e74ca5eb1b28756ce66b508d5f974dd559/stac_fastapi/types/stac_fastapi/types/core.py#L293 which is almost what we do with rio-tiler's BaseReader class. It gets complicated when we have to deal with dependency injection of the endpoints.

If you can write pseudo code, or directly a draft PR that would be awesome!

[RemiSahl]

I tried to do something here: #532
Let me know what you think, I am not sure it's the best approach but it should allow me to avoid copy-pasting the code from the gis_tools.tile function in my CustomTilerFactory.

[vincentsarago]

thanks for hard work on the PR @RemiSahl

TiTiler is a big piece of code with a lot of abstraction to other libraries so 🙏 for catching all of it.

To be honest the TilerFactories already use rio_tiler.io.BaseReader as a layer of abstraction to handle the reading, which is why we can use STACReader, COGReader and MosaicBackend with not a lot of code duplication.

The issue we are trying to solve here, is that we want to add another layer of abstraction on top of the BaseReader to replace 👇

tilesize = scale * 256

self.reader(src_path, tms=tms, **reader_params) as src_dst:
    data = src_dst.tile(
        x,
        y,
        z,
        tilesize=tilesize,
        tile_buffer=tile_buffer,
        **layer_params,
        **dataset_params,
    )
    dst_colormap = getattr(src_dst, "colormap", None)

if not format:
    format = ImageType.jpeg if data.mask.all() else ImageType.png

return image.render(
    img_format=format.driver,
    colormap=colormap or dst_colormap,
    **format.profile,
    **render_params,
)

with

return gt.tile(
    reader=self.reader,
    env=env,
    src_path=src_path,
    reader_params=reader_params,
    z=z,
    x=x,
    y=y,
    tms=tms,
    scale=scale,
    format=format,
    tile_params={**layer_params, **dataset_params},
    postprocess_params=postprocess_params,
    colormap=colormap,
    render_params=render_params,
    tile_buffer=tile_buffer,
)

Looking at ☝️ I'm not sure it's worth it!

IMO, TiTiler is really complex, adding another layer of abstraction might be too much.

I don't want to be too negative here, I really appreciate the effort and I do need to thing more about this 🙏

[RemiSahl]

Hello Vincent, and thank you for the feedback! I agree that the call to gt.tile is really verbose which bothered me as well.
Maybe one way to fix this would be to "package" the parameters in objects, kind of like what was done on layer_bands.
reader, env, src_path, and reader_params could go in a ReaderConfig object, and the remaining parameters would go in a TileConfig object. This would reduce the function calls to gt to 4 lines of codes.

I understand your concerns and I am not sure either which is the best approach. However, it makes more sense to me if you look at functions like the geojson_statistics, which is duplicated three times in factory.py! I feel like figuring out the proper way of implementing this abstraction would reduce complexity of the factory.py file which mixes up fastapi and rasterio concerns.

That being said, I am also happy with the other fix you proposed, using tile_routes as an argument of the register function!

[vincentsarago]

we could also do something like

    def tile(self, tile_routes: List[str]):
        """Register /tiles endpoint."""

        def tile(
            z: int = Path(..., ge=0, le=30, description="TMS tiles's zoom level"),
            x: int = Path(..., description="TMS tiles's column"),
            y: int = Path(..., description="TMS tiles's row"),
            tms: TileMatrixSet = Depends(self.tms_dependency),
            scale: int = Query(
                1, gt=0, lt=4, description="Tile size scale. 1=256x256, 2=512x512..."
            ),
            format: ImageType = Query(
                None, description="Output image type. Default is auto."
            ),
            src_path=Depends(self.path_dependency),
            layer_params=Depends(self.layer_dependency),
            dataset_params=Depends(self.dataset_dependency),
            postprocess_params=Depends(self.process_dependency),
            colormap=Depends(self.colormap_dependency),
            render_params=Depends(self.render_dependency),
            tile_buffer: Optional[float] = Query(
                None,
                gt=0,
                alias="buffer",
                title="Tile buffer.",
                description="Buffer on each side of the given tile. It must be a multiple of `0.5`. Output **tilesize** will be expanded to `tilesize + 2 * tile_buffer` (e.g 0.5 = 257x257, 1.0 = 258x258).",
            ),
            reader_params=Depends(self.reader_dependency),
            env=Depends(self.environment_dependency),
        ):
            """Create map tile from a dataset."""
            tilesize = scale * 256

            with rasterio.Env(**env):
                with self.reader(src_path, tms=tms, **reader_params) as src_dst:
                    data = src_dst.tile(
                        x,
                        y,
                        z,
                        tilesize=tilesize,
                        tile_buffer=tile_buffer,
                        **layer_params,
                        **dataset_params,
                    )
                    dst_colormap = getattr(src_dst, "colormap", None)

            if not format:
                format = ImageType.jpeg if data.mask.all() else ImageType.png

            image = data.post_process(**postprocess_params)

            content = image.render(
                img_format=format.driver,
                colormap=colormap or dst_colormap,
                **format.profile,
                **render_params,
            )

            return Response(content, media_type=format.mediatype)

        for r in tile_routes:
            self.router.add_api_route(
                path=r,
                methods=["GET"],
                endpoint=tile,
                **img_endpoint_params,
            )

def DatasetPathParams(layer: str = Path(..., description="Layer Name")) -> str:
    """Create dataset path from args"""
    ...
    return url



@dataclass
class MyTiler(TilerFactory):

    path_dependency = DatasetPathParams

    def register_routes(self):

        self.bounds()
        self.info()
        self.statistics()
        self.tilejson()
        self.wmts()
        self.point()
        self.preview()
        self.part()
        self.tile(
            [
                "/tiles/{layer}/{z}/{x}/{y}",
                "/tiles/{layer}/{TileMatrixSetId}/{z}/{x}/{y}",
            ]
        )

Not really ✨

[geospatial-jeff]

This is really cool but tile_routes should have a default. And maybe validation on the path parameters passed to tile_routes to make sure they match the function signature.

[vincentsarago]

@RemiSahl With the latest version of titiler (master) I've tried to reduce as much as possible to code (e.g removing timings).

I gave more though to you PR, I see the advantage of extracting the complexity outside the factory, but IMO it only solve the issue for the route path, if someone wanted to also change the dependency then they will have to change 2 block of code (in the factory and create another gis_tool method). IMO we are better off keeping everything in one place even if it means you have to re-write bunch of code :-(

[vincentsarago]

FYI: I just found a nice way to add more Path dependencies to a whole factory: stac-utils/titiler-pgstac#73 (comment)

In ☝️ we add /collections/{collection_id}/items/{item_id} as router_prefix then use

def ItemPathParams(
    request: Request,
    collection_id: str = Path(..., description="STAC Collection ID"),
    item_id: str = Path(..., description="STAC Item ID"),
) -> Dict:

as Path param (instead of url=)