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

Add ability to make array close optional #630

Merged
merged 3 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions R/TileDBArray.R
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
#' \code{/dev/shm}) for writing out results buffers (for internal use / testing)
#' @slot buffers An optional list with full pathnames of shared memory buffers to read data from
#' @slot strings_as_factors An optional logical to convert character columns to factor type
#' @slot keep_open An optional logical to not close after read or write
#' @slot ptr External pointer to the underlying implementation
#' @exportClass tiledb_array
setClass("tiledb_array",
Expand All @@ -85,6 +86,7 @@ setClass("tiledb_array",
return_as = "character",
query_statistics = "logical",
strings_as_factors = "logical",
keep_open = "logical",
sil = "list",
dumpbuffers = "character",
buffers = "list",
Expand Down Expand Up @@ -135,6 +137,7 @@ setClass("tiledb_array",
#' \sQuote{query_statistics} of the return object.
#' @param strings_as_factors An optional logical to convert character columns to factor type; defaults
#' to the value of \code{getOption("stringsAsFactors", FALSE)}.
#' @param keep_open An optional logical to not close after read or write
#' @param sil optional A list, by default empty to store schema information when query objects are
#' parsed.
#' @param dumpbuffers An optional character variable with a directory name (relative to
Expand Down Expand Up @@ -163,14 +166,15 @@ tiledb_array <- function(uri,
return_as = get_return_as_preference(),
query_statistics = FALSE,
strings_as_factors = getOption("stringsAsFactors", FALSE),
keep_open = FALSE,
sil = list(),
dumpbuffers = character(),
buffers = list(),
ctx = tiledb_get_context()) {
stopifnot(`Argument 'ctx' must be a tiledb_ctx object` = is(ctx, "tiledb_ctx"),
`Argument 'uri' must be a string scalar` = !missing(uri) && is.scalar(uri, "character"),
`At most one argument of as.data.frame, as.matrix and as.array can be selected` = sum(as.data.frame, as.matrix, as.array) <= 1,
`Argument 'as.matrix' cannot be selected for sparse arrays` = !(isTRUE(is.sparse) && as.matrix))
stopifnot("Argument 'ctx' must be a tiledb_ctx object" = is(ctx, "tiledb_ctx"),
"Argument 'uri' must be a string scalar" = !missing(uri) && is.scalar(uri, "character"),
"At most one argument of as.data.frame, as.matrix and as.array can be selected" = sum(as.data.frame, as.matrix, as.array) <= 1,
"Argument 'as.matrix' cannot be selected for sparse arrays" = !(isTRUE(is.sparse) && as.matrix))
query_type <- match.arg(query_type)
spdl::debug("[tiledb_array] query is {}", query_type)
if (sum(as.data.frame, as.matrix, as.array) == 1 && return_as != "asis")
Expand Down Expand Up @@ -205,7 +209,7 @@ tiledb_array <- function(uri,
}
}
is.sparse <- is_sparse_status
array_xptr <- libtiledb_array_close(array_xptr)
if (!keep_open) array_xptr <- libtiledb_array_close(array_xptr)
new("tiledb_array",
ctx = ctx,
uri = uri,
Expand All @@ -226,6 +230,7 @@ tiledb_array <- function(uri,
return_as = return_as,
query_statistics = query_statistics,
strings_as_factors = strings_as_factors,
keep_open = keep_open,
sil = sil,
dumpbuffers = dumpbuffers,
buffers = buffers,
Expand Down Expand Up @@ -307,6 +312,7 @@ setMethod("show", signature = "tiledb_array",
," return_as = '", object@return_as, "'\n"
," query_statistics = ", if (object@query_statistics) "TRUE" else "FALSE", "\n"
," strings_as_factors = ", if (object@strings_as_factors) "TRUE" else "FALSE", "\n"
," keep_open = ", if (object@keep_open) "TRUE" else "FALSE", "\n"
,sep="")
})

Expand Down Expand Up @@ -443,6 +449,11 @@ setValidity("tiledb_array", function(object) {
msg <- c(msg, "The 'strings_as_factors' slot does not contain a logical value.")
}

if (!is.logical(object@keep_open)) {
valid <- FALSE
msg <- c(msg, "The 'keep_open' slot does not contain a logical value.")
}

if (valid) TRUE else msg

})
Expand Down Expand Up @@ -961,7 +972,7 @@ setMethod("[", "tiledb_array",

## close array
if (status == "COMPLETE") {
libtiledb_array_close(arrptr)
if (!x@keep_open) libtiledb_array_close(arrptr)
.pkgenv[["query_status"]] <- status
finished <- TRUE
}
Expand Down Expand Up @@ -1410,7 +1421,7 @@ setMethod("[<-", "tiledb_array",
}

qryptr <- libtiledb_query_submit(qryptr)
libtiledb_array_close(arrptr)
if (!x@keep_open) libtiledb_array_close(arrptr)

}
invisible(x)
Expand Down
10 changes: 10 additions & 0 deletions inst/tinytest/test_tiledbarray.R
Original file line number Diff line number Diff line change
Expand Up @@ -1570,3 +1570,13 @@ sch <- tiledb::schema(arr)
ver <- tiledb_array_schema_version(sch)
expect_true(inherits(ver, "integer"))
expect_true(is.finite(ver))

## check for optional 'keep open'
uri <- tempfile()
fromDataFrame(mtcars, uri)
arr <- tiledb_array(uri)
res <- arr[]
expect_false(tiledb_array_is_open(arr))
arr <- tiledb_array(uri, keep_open=TRUE)
res <- arr[]
expect_true(tiledb_array_is_open(arr))
Copy link
Member

Choose a reason for hiding this comment

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

Could also check expect_false(...is_open) after manually closing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going to sneak that into #631 now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commit fb46da2

2 changes: 2 additions & 0 deletions man/tiledb_array-class.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/tiledb_array.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading