squidpy icon indicating copy to clipboard operation
squidpy copied to clipboard

Use the correct image for scatter plots with alternate spatial bases

Open obriencshl opened this issue 6 months ago • 5 comments

Description

The current assumption in the spatial scatter plotting code is that any alternate spatial bases (i.e. non-default spatial_keys) will use the images stored in the default spatial basis (adata.uns["spatial"]). My workflow involves creating an alternate spatial basis (adata.obsm["spatial_rot"]) with transformed images (to align the spatial orientations of different libraries) stored in its own spatial uns key (adata.uns["spatial_rot"]), and this assumption means I have to explicitly retrieve and pass in images to the spatial_scatter function, which is inelegant.

I am unsure if this way of using alternate spatial bases is standard or desired by others, but from my perspective, if you're using an modified spatial basis, then the images should also be modified, because the spatial basis and the background images should be expected to line up. Additionally, alternate spatial bases are required (by the Key.uns.library_mapping function, called just above my change) to have an images dict, so it seems unreasonable to have images and then not use them, regardless of the workflow.

I am not sure if scale factors etc. are properly retrieved for alternate spatial bases; my workflow does not touch the scale factors.

How has this been tested?

I have run this against my own dataset, testing against the default basis adata.obsm["spatial"] and my custom alternate basis adata.obsm["spatial_rot"] with modified images. Specifically, adata.uns["spatial_rot"] is a deep copy of adata.uns["spatial"], with the images replaced with my transformed images.

obriencshl avatar Sep 08 '25 20:09 obriencshl

Codecov Report

:x: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 24.05%. Comparing base (761a3c8) to head (8cdca3c). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/squidpy/pl/_spatial_utils.py 0.00% 2 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1032   +/-   ##
=======================================
  Coverage   24.05%   24.05%           
=======================================
  Files          43       43           
  Lines        6380     6380           
  Branches     1063     1063           
=======================================
  Hits         1535     1535           
  Misses       4828     4828           
  Partials       17       17           
Files with missing lines Coverage Δ
src/squidpy/pl/_spatial_utils.py 22.31% <0.00%> (ø)
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Sep 08 '25 20:09 codecov[bot]

Having just done some further testing, the scale factor retrieval is also incorrectly defaulting to "spatial". I'll add another commit.

obriencshl avatar Sep 08 '25 22:09 obriencshl

Segment retrieval may also be broken in scenarios like this, but I have no way of testing that at the moment, and the signatures of the functions involved don't permit an easy change.

obriencshl avatar Sep 08 '25 22:09 obriencshl

Thanks @obriencshl!

@timtreis given that our tests didn't fail for this bug (which I think is very critical because it might change the result of some studies without even noticing), do you think we should add tests here?

selmanozleyen avatar Nov 03 '25 09:11 selmanozleyen

do you think we should add tests here?

Yeah, that's a good idea, but don't overinvest. Given that the entire spatial plotting module in Squidpy will be deprecated in favor of spatialdata-plot soon, I don't think we should invest too much effort here.

I guess this is also an FYI for your mentioned work @obriencshl

timtreis avatar Nov 03 '25 15:11 timtreis