-
Notifications
You must be signed in to change notification settings - Fork 98
Use the correct image for scatter plots with alternate spatial bases #1032
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Retrieve the image from adata.uns[spatial_key] rather than always adata.uns["spatial"].
Codecov Report❌ Patch coverage is
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
🚀 New features to boost your workflow:
|
|
Having just done some further testing, the scale factor retrieval is also incorrectly defaulting to |
|
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? |
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 I guess this is also an FYI for your mentioned work @obriencshl |
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 thespatial_scatterfunction, 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_mappingfunction, called just above my change) to have animagesdict, 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 basisadata.obsm["spatial_rot"]with modified images. Specifically,adata.uns["spatial_rot"]is a deep copy ofadata.uns["spatial"], with the images replaced with my transformed images.