Skip to content

return NULL if path does not exist #196

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

Merged
merged 2 commits into from
Nov 12, 2024
Merged

return NULL if path does not exist #196

merged 2 commits into from
Nov 12, 2024

Conversation

rcannood
Copy link
Collaborator

@rcannood rcannood commented Nov 12, 2024

Without this, we get an error when reading an empty h5ad.

Example

import anndata as ad

adata = ad.AnnData(
  shape=(10, 20)
)

adata.write_h5ad("empty.h5ad")
devtools::load_all()
adata <- read_h5ad("empty.h5ad")
Error in `[[.H5File`(file, name) : 
  An object with name X does not exist in this group

Testing

This situation will be covered by #193 by removing X = ... from the dummy_anndata generate_dataset call

@rcannood rcannood requested a review from lazappi November 12, 2024 11:03
@lazappi
Copy link
Collaborator

lazappi commented Nov 12, 2024

How much have you tested this? I wonder if returning NULL could break some things, like do we use X to get n_vars and n_obs?

@rcannood
Copy link
Collaborator Author

Since X might not always be in the hdf5 file (as in, the group does not even exist), we should use it to derive values like n_obs and n_vars.

Just noticed that I made a mistake when committing my code, hopefully fixed now.

Copy link
Collaborator

@lazappi lazappi left a comment

Choose a reason for hiding this comment

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

I checked and it's obs/var that have to exist so I think this should be fine

@rcannood rcannood merged commit 3790caa into main Nov 12, 2024
7 checks passed
@rcannood rcannood deleted the fix-not-exists branch November 12, 2024 12:44
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.

2 participants