Fix: Critical updates for Seurat v5, spacexr 2.0, and robust spot alignment - #266
Fix: Critical updates for Seurat v5, spacexr 2.0, and robust spot alignment#266Atakannl wants to merge 11 commits into
Conversation
|
Thanks so much for the PR @Atakannl, I will review this one with my use cases and get back to you! Artür |
|
Dear @Atakannl, thanks so much for the PR. Can you take a look at my review and correct if can. In summary, i do not believe we should implicitly join layers of a Seurat object. I would rather mention this in the documentation/tutorial, you can attempt this for the doc/rmd of the website which is here in the package too. I would explain that people who use v5 should join layers first (I find the need to do VoltRon/docs/nicheclustering.Rmd Lines 289 to 311 in ecd8f23 |
- Removed the implicit layer joining - Added validation for multi-layer assays - Require user to run JoinLayers before deconvolution
|
Thanks for the suggestion @Artur-man . I removed the implicit JoinLayers call and now explicitly check for multi-layer assays in Seurat v5. If multiple layers are detected, the user is instructed to run JoinLayers beforehand. No automatic layer modification is performed anymore. I will also add a note to the documentation for Seurat v5 users. |
|
I added the Seurat v5 compatibility note to both RCTD sections in the niche clustering vignette. The documentation now explicitly states that users must run |
|
@Atakannl thanks so much. I have left some comments I think this PR would make VoltRon comply with Seurat v5 and recent version of spacexr. We should only make the PR a bit cleaner and reduce unnecessary if statements, otherwise I like it. If you like I can resolve the conflicts for you, then you can continue updating. |
|
That would be great, @Artur-man, thank you. I will continue refining the conditional logic once the conflicts are resolved. |
|
@Atakannl ready for you now. Let me know if you have any questions. |
|
@Atakannl just checking in, any updates on this ? would be nice to wrap this PR soon. |
|
Thanks for following up and for pushing the updates. I appreciate it. Apologies for the delay on my side. I’ll review the latest changes and continue cleaning up the conditional logic and structure so we can wrap this PR soon. |
|
No worries @Atakannl. Also for clarity let me clarify some stuff that i discussed with folks at Bioconductor. There are two implementations of We have to use So actually spacexr >=2.0.0 is not the latest version, but the legacy implementation itself (Bioconductor uses a different versioning system so it looks like version in Bioc is previous) For the Seurat version I limited the versioning of Seurat to |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 939753e48b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (length(layer_names) > 1) { | ||
| stop( | ||
| "Seurat v5 layered assay detected with multiple layers. ", | ||
| "Please run SeuratObject::JoinLayers(sc.object, assay = '", | ||
| sc.assay, |
There was a problem hiding this comment.
The length(layer_names) > 1 guard in getDeconReferenceSpot() rejects valid Seurat v5 objects that contain multiple layers (e.g., counts, data, scale.data), even though this function immediately requests layer = "counts" via GetAssayData and that usage is supported. This turns many normal Seurat workflows into hard failures and prevents deconvolution from running on otherwise compatible references.
Useful? React with 👍 / 👎.
| if (length(common) == 0 && n_target > 0) { | ||
| safe_target <- make.names(target_spots) | ||
| safe_raw <- make.names(rownames(results)) | ||
| map_idx <- match(safe_target, safe_raw) | ||
| valid_idx <- which(!is.na(map_idx)) |
There was a problem hiding this comment.
The fallback make.names mapping is only executed when there are zero exact spot-name matches, so mixed cases (some exact matches plus some sanitized-only matches) skip the fallback and leave valid spots as all-zero rows. This silently drops part of the RCTD output whenever only a subset of barcodes is transformed (for example, IDs containing - alongside IDs that were already syntactically safe).
Useful? React with 👍 / 👎.
| if (res_cols == n_target && res_rows != n_target) { | ||
| results <- t(results) |
There was a problem hiding this comment.
The transpose heuristic in getRCTD() relies on dimensions (res_cols == n_target && res_rows != n_target) instead of dimnames, so a square weights matrix (same number of cell types and spots) is treated as already spot-by-feature even when spacexr returns cell-types-by-spots. In that case the subsequent row-wise normalization runs over cell types rather than spots and produces incorrect deconvolution proportions.
Useful? React with 👍 / 👎.
|
Dear @Atakannl, just checking in. You can also let me know if you do not wanna finish the PR. Although it is not ideal, I might do some adjustments and merge it. |
|
Dear @Artur-man , apologies for the delay. I've been fully occupied with finishing my master's thesis, and the submission deadline is very close, so I haven't been able to give this PR the attention it deserves. I completely understand if you'd prefer to make the remaining adjustments and merge it yourself so it doesn't stay blocked. Thank you for all the work you've already put into it. Once my thesis is submitted, I'd be happy to contribute again to future improvements if needed. |
2 participants
Hi VoltRon team,
While utilizing getDeconvolution() for a GBM spatial analysis, I encountered breaking compatibility issues with Seurat v5 (layer handling) and spacexr 2.0+ (Reference object requirements).
This PR implements the following fixes to modernize the vrDeconvolution module while maintaining strict backward compatibility:
Seurat v5 Support: Added a check for JoinLayers to correctly handle count retrieval (layer="counts" vs slot="counts"), fixing the "slot 'counts' not found" error.
spacexr 2.0 Compliance:
Implemented the native spacexr::Reference constructor for v2.0+ users.
Updated getRCTD to use the SpatialRNA constructor with data.frame coordinates (required by v2.0).
Preserved the legacy SpatialExperiment path for users on older versions.
Robust Spot Alignment: Added logic to align RCTD results back to the original VoltRon spot IDs. This fixes issues where RCTD drops spots during QC or sanitizes barcodes (e.g., changing - to .), which previously resulted in dimension mismatches or silent data loss.
Safety Validations:
Added explicit checks for sc.cluster existence in metadata to prevent cryptic errors.
Added hard failure stops if spot alignment finds zero matches.
I have verified these fixes on a merged GBM cohort using both Seurat v5 and spacexr v2.2.1.
Thanks for the great tool!