Add Artifact$open() method
#117
Conversation
Also adjust alert types and add tests
If it works...
Use Artifact$open() to access a store instead
|
@rcannood This works but requires {tiledbsoma} which is only on R-Universe. I'm not sure how that would go with CRAN so at the moment I haven't included it in |
|
@falexwolf Can you please check if you are ok with the new example in the vignette here https://github.com/laminlabs/laminr/pull/117/files#diff-0c3baf1323c3304811b8f4f0f91c300e9242052855269e47afdb5276a7dd6c18? I tried to replicate this Python example https://docs.lamin.ai/cellxgene#slice-a-concatenated-array. |
| #' @return A [tiledbsoma::SOMACollection] or [tiledbsoma::SOMAExperiment] | ||
| #' object | ||
| open = function() { | ||
| is_tiledbsoma <- private$get_value("suffix") == ".tiledbsoma" || |
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok. I'll have to see if we have access to that.
| extra_repos = "https://chanzuckerberg.r-universe.dev" | ||
| ) | ||
|
|
||
| artifact_uri <- paste0( |
There was a problem hiding this comment.
The problem is that it won't work for s3 paths i think. At least the correct region should be provided, we have additional setup here https://github.com/laminlabs/lamindb/blob/39e0f529a41d9cbc475e52fe3aa0b8095af21494/lamindb/core/storage/_tiledbsoma.py#L33 but not sure what is available in R.
| ```{r slice-tiledbsoma, eval = requireNamespace("tiledbsoma", quietly = TRUE)} | ||
| # Set some environment variables to avoid an issue with {tiledbsoma} | ||
| # https://github.com/chanzuckerberg/cellxgene-census/issues/1261 | ||
| Sys.setenv(TILEDB_VFS_S3_REGION = "us-west-2") |
There was a problem hiding this comment.
aha, ok, the region is provided here, i see. But it is probably much better to provide via tiledb context.
There was a problem hiding this comment.
This was just a hacky workaround because there is a bug in the {tiledbsoma} package. I'll look into the context stuff but I'm not sure if we get all the necessary information from the API.
There was a problem hiding this comment.
I think that work-around is OK for now given you clearly marked it in the comments!
Sergei can help find a cleaner solution over the next weeks & months; it's not so urgent yet that this is 100% polished.
I think this is good to merge!
Related to: Fixes #114
Description
Artifact$open()method to access TileDB-SOMA storescheck_requires()to handle extra repositories (i.e. R-Universe)Artifact$open()Checklist
Before review
Before merge
READMECHANGELOG