spatialdata-plot icon indicating copy to clipboard operation
spatialdata-plot copied to clipboard

Multiscale images lose channel names

Open aeisenbarth opened this issue 1 year ago • 1 comments

Rendering commands that take channels fail when you pass channel names instead of indices.

Example

from spatialdata import SpatialData
from spatialdata.models import Image2DModel
import numpy as np
import spatialdata_plot

# Two channels: gradient left-right, gradient top-down
array = np.stack(np.meshgrid(np.linspace(0, 1, 3), np.linspace(0, 1, 3)))
sdata = SpatialData(
    images={"image1": Image2DModel.parse(array, dims=("c", "y", "x"), c_coords=["DAPI", "GFP"], scale_factors=[2])}
)
sdata.pl.render_images(elements="image1", channel=["DAPI", "GFP"]).pl.show()
# File "…/spatialdata-plot/src/spatialdata_plot/pl/render.py", line 420, in _render_images
#    layers[c] = img.sel(c=c).copy(deep=True).squeeze()
# …
# KeyError: "not all values found in index 'c'. Try setting the `method` keyword argument (example: method='nearest')."

This works correctly for single-scale images, but fails for multiscale. However, indices work also for multiscale:

sdata.pl.render_images(elements="image1", channel=[0, 1]).pl.show()

This is caused by _multiscale_to_spatial_image calling Image2DModel.parse(image) without passing all arguments.

I've encountered bugs like this frequently, and I think besides fixing the above call, we should do:

  • completely testing the whole code bases (including spatialdata) for both single-scale and multiscale. Best by parametrizing every test with @pytest.mark.parametrize("image_cls", [SpatialImage, MultiscaleSpatialImage]) or @pytest.mark.parametrize("scale_factors", [None, [2]])
  • extend model classes to have a "copy constructor" (EDIT: shallow, not deep of course). Currently, calling Image2DModel.parse with a SpatialImage interprets it as an array, not as a SpatialImage.
  • redesign SpatialImage and MultiscaleSpatialImage so they can be used interchangeably without error-prone special handling (like _multiscale_to_spatial_image)

aeisenbarth avatar Dec 14 '23 15:12 aeisenbarth

Thanks for reporting, for the moment I would add the fix to the call of Image2DModel here in spatialdata plot.

Regarding the other points.

completely testing the whole code bases (including spatialdata) for both single-scale and multiscale

We are definitely going to improve the tests to cover the multiscale cases more systematically (today we finished a PR doing this for the query APIs), and if you have specific instances we can prioritize them. I also agree on the important on this for this repository.

extend model classes to have a "copy constructor"

While I think a real copy constructor would be not ideal when the data is large, I also agree that the parser should accept the final data type it gives as return. This is actually already the case for the other parsers, but you are right it fails with SpatialImage because of the ambiguity with DataArray. See more below.

redesign SpatialImage and MultiscaleSpatialImage

I think that the classes became more interoperable with the last release (that unfortunately we haven't adopted yet: https://github.com/scverse/spatialdata/issues/395). We want to do it after we close some open PRs.

LucaMarconato avatar Jan 16 '24 13:01 LucaMarconato