Skip to content

Do not finalize read queries#309

Merged
eddelbuettel merged 1 commit intomasterfrom
de/sc-9988/s3_vs_tiledb
Oct 15, 2021
Merged

Do not finalize read queries#309
eddelbuettel merged 1 commit intomasterfrom
de/sc-9988/s3_vs_tiledb

Conversation

@eddelbuettel
Copy link
Copy Markdown
Contributor

The one-line change below is the difference between whether an identical query sent to one of

  • s3://genomic-datasets/biological-databases/data/tables/gtex-analysis-rnaseqc-gene-tpm
  • tiledb://TileDB-Inc/gtex-analysis-rnaseqc-gene-tpm

succeeds in both cases. Without the change it will not work for the second uri.

A simple test script follows, Calling with any one of b or bad or tile will show the issues---by failing to retrieve data---unless the change in the PR is made.

#!/usr/bin/Rscript

library(tiledb)
argv <- commandArgs(TRUE)

## good
uri <- "s3://genomic-datasets/biological-databases/data/tables/gtex-analysis-rnaseqc-gene-tpm"

## bad
if (length(argv) > 0 && is.finite(match(argv, c("b", "bad", "tile"))))
    uri <- "tiledb://TileDB-Inc/gtex-analysis-rnaseqc-gene-tpm"

cat("Using uri", uri, "\n")
gtex_array <- tiledb_array(uri = uri, is.sparse = TRUE, attrs = "tpm", return_as = "tibble")
res <- gtex_array["ENSG00000199405.1", ]
print(res)

@shortcut-integration
Copy link
Copy Markdown

Copy link
Copy Markdown
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

Pls remove the line and update the comment as discussed, otherwise LGTM. Good catch!

@eddelbuettel eddelbuettel force-pushed the de/sc-9988/s3_vs_tiledb branch from 38f5865 to 347664c Compare October 14, 2021 20:30
@eddelbuettel eddelbuettel changed the title Do not finalize query Do not finalize read queries Oct 14, 2021
@eddelbuettel eddelbuettel requested a review from ihnorton October 14, 2021 20:31
@eddelbuettel eddelbuettel merged commit f013f9b into master Oct 15, 2021
@eddelbuettel eddelbuettel deleted the de/sc-9988/s3_vs_tiledb branch October 15, 2021 13:08
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