Skip to content
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

[r] [WIP] Add support for reading v5 assays from an axis query #3008

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mojaveazure
Copy link
Member

Seurat v5 adds support for ragged arrays, where not every X layer has exactly the same cells and features. To handle this, ragged X layers need to be re-indexed and re-shaped on ingestion to resize down to only the data present

Modified SOMA methods:

  • SOMAExperimentAxisQuery$to_seurat() and SOMAExperimentAxisQuery$to_seurat_assay(): now read in as v5 assays

New SOMA methods:

  • SOMAExperimentAxisQuery$private$.to_seurat_assay_v5(): helper method to read in ragged and non-ragged arrays into a v5 assay; note this method only handles expression layers, all other assay-level information is handled by parent $to_seurat_assay() to share code with v3 assay outgestion

Requires #2523 and #3007

SC-52261

Seurat v5 adds support for ragged arrays, where not every `X` layer has
exactly the same cells and features. To handle this, ragged `X` layers
need to be re-indexed and re-shaped on ingestion to resize down to only
the data present

Modified SOMA methods:
 - `SOMAExperimentAxisQuery$to_seurat()` and
   `SOMAExperimentAxisQuery$to_seurat_assay()`: now read in as v5 assays

New SOMA methods:
 - `SOMAExperimentAxisQuery$private$.to_seurat_assay_v5()`: helper
   method to read in ragged and non-ragged arrays into a v5 assay; note
   this method only handles expression layers, all other assay-level
   information is handled by parent `$to_seurat_assay()` to share code
   with v3 assay outgestion

Requires #2523 and #3007

[SC-52261](https://app.shortcut.com/tiledb-inc/story/52261/)
@mojaveazure mojaveazure force-pushed the paulhoffman/sc-52261/add-support-for-reading-ragged-arrays-from branch from 8f13006 to 575c12b Compare November 5, 2024 19:24
@mojaveazure mojaveazure marked this pull request as ready for review November 5, 2024 21:58
Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes from initial readability pass

@@ -767,7 +775,23 @@ SOMAExperimentAxisQuery <- R6::R6Class(
"'drop_levels' must be TRUE or FALSE" = isTRUE(drop_levels) ||
isFALSE(drop_levels)
)
match.arg(version, choices = "v3")
assay_hint <- names(.assay_version_hint())
# Get the assay version
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Get the assay version
# Get the Seurat assay version

check_package('SeuratObject', version = '5.0.2')

# Create dummy layer to initialize v5 assay
lname <- SeuratObject::RandomName(length = 7L)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lname <- SeuratObject::RandomName(length = 7L)
layer_name <- SeuratObject::RandomName(length = 7L)

)

# Read in layers
for (lyr in layers) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (lyr in layers) {
for (layer_name in layer_names) {

}
}
}
lcells <- cells
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lcells <- cells
layer_cells <- cells

}
}
lcells <- cells
lfeatures <- features
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lfeatures <- features
layer_features <- features

if (is.null(tbl)) {
next
}
sdx <- vector("list", length = length(dnames))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what sdx means. Please use a longer name that indicates what this means.


obsv <- seq.int(to = n_obs)
varv <- seq.int(to = n_var)
nd <- seq(from = 0L, to = 1L, by = 0.1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nd = number of dimensions? Please spell out with an informative name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants