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

Empty tables and nullable #17

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

Empty tables and nullable #17

wants to merge 2 commits into from

Conversation

gtca
Copy link
Contributor

@gtca gtca commented Nov 28, 2023

Should make it possible to read more files out there as in #14

- Makes it possible to handle empty tables (e.g. 0 features)
- Adds notion of nullable-integer and nullable-boolean
@@ -40,7 +40,11 @@ read_table_encv1 <- function(dataset, set_index = TRUE) {
columns <- columns[columns != "__categories"]

col_list <- lapply(columns, function(name) {
values <- dataset[[name]]$read()
if (dataset[[name]]$dims == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps add a comment referencing hhoeflin/hdf5r#118 so we know what this workaround is for (and can remove it in the future)

Copy link
Collaborator

@ilia-kats ilia-kats left a comment

Choose a reason for hiding this comment

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

LGTM

@DriesSchaumont
Copy link

Thanks for the PR! 🙇 When running this branch on s3://openpipelines-data/10x_5k_anticmv/5k_human_antiCMV_T_TBNK_connect_mms.h5mu as described at #14 I now get the following error. I don't know if this is related to the original issue or if I should creata a new ticket. Let me know!
I appreciate you guys pointing out that one of the modalities has 0 features. I did not notice before 🤔

Error in `LayerData()`:
! `features` must be a character vector, not `NULL`.
Backtrace:
     ▆
  1. └─MuDataSeurat::ReadH5MU(par$input)
  2.   ├─Seurat::CreateSeuratObject(...)
  3.   └─SeuratObject:::CreateSeuratObject.Assay(...)
  4.     ├─SeuratObject:::CalcN(object = counts)
  5.     └─SeuratObject:::.CalcN.Assay(object = counts)
  6.       ├─SeuratObject::LayerData(object = object, layer = layer)
  7.       └─SeuratObject:::LayerData.Assay(features = NULL)
  8.         └─rlang::arg_match(...)
  9.           └─rlang:::check_character(arg, arg = error_arg, call = error_call)
 10.             └─rlang:::stop_input_type(...)
 11.               └─rlang::abort(message, ..., call = call, arg = arg)

@gtca
Copy link
Contributor Author

gtca commented Dec 2, 2023

Hey @DriesSchaumont,

Which version of Seurat / SeuratObject have you been using?
With Seurat_4.0.4 and SeuratObject_4.0.2 I cannot reproduce it but I suspect you see it in Seurat 5?

We haven't covered Seurat 5 changes yet — the respective changes should probably land in another PR. A new issue might be a good start? 😃

Do you think the error you see here is also related to a modality with no features?..

@DriesSchaumont
Copy link

DriesSchaumont commented Dec 11, 2023

Hi @gtca, that worked (although I could not use Seurat 4.4.0). Seurat v5 support would be great, but as you said: that would be for another PR 👍

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

Successfully merging this pull request may close these issues.

3 participants