fix: match Seurat key sanitization when detecting assay-specific reductions - #3
Merged
Merged
Conversation
…ctions Reductions whose names contain non-alphanumeric characters were silently dropped from .obsm on export. Seurat strips those characters when deriving a key from a reduction name (umap_harmony -> umapharmony_), but the modality-specific check only stripped dots, so the name never matched its own key and the reduction failed every branch of the test. On a single-assay object this dropped umap_harmony, pca_harmony, pca_uncorrected and mrVI_umap while keeping umap, pca and harmony. Embeddings without feature loadings had no other way to be recognised. - strip all non-alphanumerics instead of only dots, in both WriteH5ADHelper and the parallel block in WriteH5MU - add fixed = TRUE: reduction names were being treated as regex patterns - warn instead of silently skipping, so a future mismatch is visible Note for WriteH5MU: these reductions were previously misclassified as multimodal and written to the global /obsm. They now correctly land in /mod/<assay>/obsm.
There was a problem hiding this comment.
Pull request overview
This PR fixes export-time detection of assay-specific reductions by matching Seurat’s key sanitization rules, preventing reductions with non-alphanumeric characters in their names from being silently omitted or misrouted during WriteH5AD/WriteH5MU export.
Changes:
- Sanitize reduction names by stripping all non-alphanumeric characters (instead of only
.) when comparing reduction names to Seurat-derived keys. - Use
grepl(..., fixed = TRUE)to avoid interpreting arbitrary reduction names as regex patterns. - Add a warning when a reduction is not recognized as assay-specific in
WriteH5ADHelper, instead of silently skipping it.
Comments suppressed due to low confidence (1)
R/WriteH5MU.R:422
- Same as above: if
gsub('[^[:alnum:]]', '', red_name)yields an empty string,grepl('', red@key, fixed=TRUE)will always match and can misclassify the reduction as modality-specific. Add annzchar()guard (or equivalent).
if (grepl(tolower(assay_emb), tolower(red_name), fixed = TRUE) ||
grepl(tolower(assay_emb), tolower(red@key), fixed = TRUE) ||
grepl(tolower(red_name), tolower(red@key), fixed = TRUE) ||
grepl(tolower(gsub('[^[:alnum:]]', '', red_name)), tolower(red@key), fixed = TRUE)
) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+164
to
168
| if (grepl(tolower(emb_assay), tolower(red_name), fixed = TRUE) || | ||
| grepl(tolower(emb_assay), tolower(red@key), fixed = TRUE) || | ||
| grepl(tolower(red_name), tolower(red@key), fixed = TRUE) || | ||
| grepl(tolower(gsub('[^[:alnum:]]', '', red_name)), tolower(red@key), fixed = TRUE) | ||
| ) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Reductions whose names contain non-alphanumeric characters were silently dropped from
.obsmon export.Seurat strips non-alphanumerics when deriving a key from a reduction name (
umap_harmony→ keyumapharmony_), but the modality-specific check inWriteH5ADHelperonly stripped dots —gsub('\\.', '', red_name). The sanitized name therefore still contained the underscore that the key no longer had, so the reduction failed every branch of the test, hit thenext, and was skipped without any warning.Embeddings that carry
feature.loadings(e.g.pca) survived via the earlier loadings-based check. UMAP-style embeddings have no loadings, so they had no other way to be recognised.Reproduced on a real single-assay object with
WriteH5AD(..., assay = 'RNA'):The dropped set is exactly the reductions whose names contain a character Seurat removes from the key.
Changes
All in
R/WriteH5MU.R:[^[:alnum:]]) instead of only dots, so the comparison matches Seurat's own key derivation. Applied in bothWriteH5ADHelperand the parallel block inWriteH5MUso the two paths stay in agreement.fixed = TRUEto thegreplcalls — reduction names were being interpreted as regex patterns, which is wrong for arbitrary user-chosen names.nextwith awarning()naming the reduction, its key, and itsassay.used, so a future mismatch is visible rather than producing a quietly incomplete file.Behavior change to review
In
WriteH5MU, these four reductions were previously misclassified as multimodal and written to the global/obsm. They are now correctly recognised as assay-specific and land in/mod/<assay>/obsm.This is the intended placement, but it is a breaking change for any downstream code reading these embeddings from the top-level
obsmof an.h5mu.Test plan
devtools::test()— 57 pass, 0 fail, 0 warnWriteH5AD(assay = 'RNA'); all 10 reductions now present in.obsm(was 6)anndata:X_umap_harmonyis(58122, 2)float64, all finite, values match the R-sidecell.embeddingsexactly; cell order matchescolnames(srt)WriteH5MUround-trip viamudata.read_h5mu— all 10 undermod['RNA'].obsm, globalobsmholds only theRNAmod mapobsmplacement