Use the correct image for scatter plots with alternate spatial bases
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.
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.
Having just done some further testing, the scale factor retrieval is also incorrectly defaulting to "spatial". I'll add another commit.
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.
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?
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