You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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)
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: scverse/spatialdata#395). We want to do it after we close some open PRs.
The current behaviour is limited to directly using the correct channel names that exist in the data. If you accidentally specify anything that doesn't exist, it'll tell you the valid options. I don't think we'll additionally need the option to pick them by their integer-order 🤔
Activity
LucaMarconato commentedon Jan 16, 2024
Thanks for reporting, for the moment I would add the fix to the call of
Image2DModelhere in spatialdata plot.Regarding the other points.
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.
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
SpatialImagebecause of the ambiguity withDataArray. See more below.I think that the classes became more interoperable with the last release (that unfortunately we haven't adopted yet: scverse/spatialdata#395). We want to do it after we close some open PRs.
timtreis commentedon Mar 29, 2026
The current behaviour is limited to directly using the correct channel names that exist in the data. If you accidentally specify anything that doesn't exist, it'll tell you the valid options. I don't think we'll additionally need the option to pick them by their integer-order 🤔