Skip to content

Add helper to test if an array is open#421

Merged
aaronwolen merged 6 commits intomasterfrom
aaronwolen/add-array-open-helper
Jun 3, 2022
Merged

Add helper to test if an array is open#421
aaronwolen merged 6 commits intomasterfrom
aaronwolen/add-array-open-helper

Conversation

@aaronwolen
Copy link
Copy Markdown
Member

Adds the array-based complement to the helpful tiledb_group_is_open() function.

Comment thread R/Array.R Outdated
##' @return A boolean indicating whether the TileDB Array object is open
##' @export
tiledb_array_is_open <- function(arr) {
stopifnot(`The 'arr' argument must be a tiledb_array object` = is(arr, "tiledb_array") || is(arr, "tiledb_sparse") || is(arr, "tiledb_dense"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe

 stopifnot("The 'arr' argument must be a tiledb_array object" = .isArray(arr))

as I am sloooowly changing to use double-quotes for named first argument, and we (for a long time) had that handy helper (which I also sometimes forget to use). Just a nit, almost not worth a commit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Totally worth a commit! I was thinking a utility would be helpful for such a common pattern. As of b9df9c1 it's used everywhere in Array.R. Thanks for the hint!

@eddelbuettel
Copy link
Copy Markdown
Contributor

Nice addition. I guess this was overlooked as an export-to-R when the library function was wrapped.

@aaronwolen aaronwolen marked this pull request as ready for review June 3, 2022 18:40
Copy link
Copy Markdown
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

👍

@aaronwolen aaronwolen merged commit 5cb3e90 into master Jun 3, 2022
@eddelbuettel
Copy link
Copy Markdown
Contributor

Some additional tests blew up over line 993 in inst/tinytest/test_tiledbarray.R which has an accidental upper-case T:

expect_True(tiledb_array_is_open(arr)

I'll clean that up when I do 2.9.3 tomorrow.

@aaronwolen
Copy link
Copy Markdown
Member Author

Ah dangit, sorry about that! Why didn't CI choke?

@eddelbuettel
Copy link
Copy Markdown
Contributor

I am not sure. Most likely that the remainder of that test file got skipped because a data package was not there. I have an auxiliary repo I use for CI against all other 2.* release, dev, and some more -- and they all went belly-up.

It happens to me at times too. A little tedious when it happens but it keeps the day-to-day, commit-to-commit CI a little quicker so I am happy to pay the price. I fixed it in that repo with a one char commit.

@eddelbuettel eddelbuettel mentioned this pull request Jun 4, 2022
@eddelbuettel eddelbuettel deleted the aaronwolen/add-array-open-helper branch June 6, 2022 19:58
@eddelbuettel eddelbuettel mentioned this pull request Jun 23, 2022
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