XarrayReader/Reader conflicting parameters

[vincentsarago]

titiler-stacapi-mspc/titiler/stacapi/factory.py

Lines 198 to 225 in 3b80678

	with rasterio.Env(**env):
	with self.reader(
	url=api_params["api_url"],
	headers=api_params.get("headers", {}),
	tms=tms,
	reader_options={**reader_params},
	**backend_params,
	) as src_dst:
	if MOSAIC_STRICT_ZOOM and (
	z < src_dst.minzoom or z > src_dst.maxzoom
	):
	raise HTTPException(
	400,
	f"Invalid ZOOM level {z}. Should be between {src_dst.minzoom} and {src_dst.maxzoom}",
	)
	image, assets = src_dst.tile(
	x,
	y,
	z,
	search_query=search_query,
	tilesize=scale * 256,
	pixel_selection=pixel_selection,
	threads=MOSAIC_THREADS,
	**tile_params,
	**layer_params,
	**dataset_params,
	)

recap:

In titiler-pgstac/stacfastapi tilers we have 3 levels of reader. One to access the Mosaic (backend), one to access the Item and then a last one to access the data itself.

• backend: pgstac or stacapi mosaic backend, will merge request to different item reader
• reader (item): STAC item reader, will merge request to different asset reader
• reader (asset): Xarray or Raster dataset reader

Dependency unpacking issue

Most of the dependencies are defined using DefaultDependency which is then unpacked (**dep) within the endpoints. This worked well because usually all the asset's reader are all the same, and thus when we define the reader in the backend we know which options can be passed. The issue we have right now, is that both rio_tiler.io.xarray.XarrayReader and rio_tiler.io.rasterio.Reader are quite different and we are trying to use them within the same tiler factory, so it's really hard because we now have set of QueryParameters which can be passed to one reader but not the other (e.g the variable init xarray reader parameter).

# titiler.core.dependencies. DefaultDependency
@dataclass
class DefaultDependency:
    """Dataclass with dict unpacking"""

    def keys(self):
        """Return Keys."""
        return self.__dict__.keys()

    def __getitem__(self, key):
        """Return value."""
        return self.__dict__[key]

class dep(DefaultDependency):
    value: str = Query("value")
    value2: str = Query("value2")

@get("/yo")
def yo(params=Depends(dep):
    return func(**params)

• backend.reader.reader_options (L203), will get values from reader_params defined with reader_dependency.

The reader_options is then forwarded to the Asset's reader

titiler-stacapi-mspc/titiler/stacapi/backend.py

Line 299 in 3b80678

with self.reader(item, tms=self.tms, **self.reader_options) as src_dst:

It default to DefaultDependency so nothing will be passed to the Asset reader. But for XarrayReader we will want to have variable (and more https://github.com/developmentseed/titiler-xarray/blob/dev/titiler/xarray/factory.py#L84-L121).

If we create a dependency like

class dep(DefaultDependency):
    variable: Optional[str] = Query(None)

⚠️ This will pass variable=None to the rasterio Reader and errors because variable= is not a Rasterio's reader option.

• tile_params (L222), defined with tile_dependency and default to TileParams https://github.com/developmentseed/titiler/blob/main/src/titiler/core/titiler/core/dependencies.py#L617-L637

⚠️ buffer and padding options are not available in the XarrayReader.tile method (https://github.com/cogeotiff/rio-tiler/blob/main/rio_tiler/io/xarray.py#L200-L210)

• layer_params (L223), defined with layer_dependency and default to AssetsBidxExprParams (defined in the Factory). This will be used both by the Item's reader (assets=["asset1"]) and the Asset's reader (asset_bidx=["asset1|1,2,3"])

⚠️ indexes option is not in the XarrayReader.tile method (https://github.com/cogeotiff/rio-tiler/blob/main/rio_tiler/io/xarray.py#L200-L210)

• dataset_params (L224), defined with dataset_dependency and default to DatasetParams. This will be forwarded to the asset's reader

⚠️ unscale option is not in the XarrayReader.tile method

Solution

To avoid conflicting parameters to be passed, we are proposing a change in titiler developmentseed/titiler#819

This PR will change the way we pass the dependency values to the reader method, by not using unpacking but a new .kwargs method which will only pass values not set to None (e.g variable= will only be passed if set by the user).

This doesn't avoid parameter conflict because the endpoint has not idea which asset's reader will be used (because it will be defined dynamically within the Item's reader).

If you read this line, consider yourself a titiler expert now 🙏

cc @abarciauskas-bgse

[abarciauskas-bgse]

Thanks @vincentsarago that makes sense to me. I will have a closer look tomorrow but it seems like this is something we can implement in this repo so we are not waiting on the updated titiler.

[vincentsarago]

🥳

FYI, we don't need to re-implement the new .kwargs method here but we can simply do something like

            
            reader_options = {
                k: v
                for k, v in reader_params.__dict__.items()
                if v is not None
            }
            backend_options = {
                k: v
                for k, v in backend_params.__dict__.items()
                if v is not None
            }
            tile_options = {
                k: v
                for k, v in tile_params.__dict__.items()
                if v is not None
            }
            layer_options = {
                k: v
                for k, v in layer_params.__dict__.items()
                if v is not None
            }
            dataset_options = {
                k: v
                for k, v in dataset_params.__dict__.items()
                if v is not None
            }

            with rasterio.Env(**env):
                with self.reader(
                    url=api_params["api_url"],
                    headers=api_params.get("headers", {}),
                    tms=tms,
                    reader_options=reader_options,
                    **backend_options,
                ) as src_dst:
                    if MOSAIC_STRICT_ZOOM and (
                        z < src_dst.minzoom or z > src_dst.maxzoom
                    ):
                        raise HTTPException(
                            400,
                            f"Invalid ZOOM level {z}. Should be between {src_dst.minzoom} and {src_dst.maxzoom}",
                        )

                    image, assets = src_dst.tile(
                        x,
                        y,
                        z,
                        search_query=search_query,
                        tilesize=scale * 256,
                        pixel_selection=pixel_selection,
                        threads=MOSAIC_THREADS,
                        **tile_options,
                        **layer_options,
                        **dataset_options,
                    )

[abarciauskas-bgse]