Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

tiledb_config uses vectorized is.na in if #518

Closed
mojaveazure opened this issue Feb 13, 2023 · 1 comment · Fixed by #519
Closed

tiledb_config uses vectorized is.na in if #518

mojaveazure opened this issue Feb 13, 2023 · 1 comment · Fixed by #519

Comments

@mojaveazure
Copy link
Member

In R 4.2, if() with a condition of length greater than one results in an error. As such, calling tiledb_config() with a vector of length greater than one will error out

> tiledb_config(c(sm.consolidation.amplification = '1.1', sm.mem.reader.sparse_global_order.ratio_array_data = '0.3'))
Error in if (!is.na(config)) { : the condition has length > 1
@eddelbuettel
Copy link
Contributor

Nice catch! I guess if (!any(is.na(config))) should help.

mojaveazure added a commit that referenced this issue Feb 13, 2023
@mojaveazure
Safer check for NAs in tiledb_config()
d1cc7ae 
Replace `if(is.na())` as R 4.2 errors when conditions > 1L
Instead, removes NAs from `config`
Then, checks the length of `config`; if there is a length, adds it to
the TileDB config

fixes #518
@mojaveazure mojaveazure mentioned this issue Feb 13, 2023
Merged
mojaveazure added a commit that referenced this issue Feb 14, 2023
@mojaveazure
Safer check for NAs in tiledb_config() (#519)
6abcd6e 
* Safer check for NAs in `tiledb_config()`
Replace `if(is.na())` as R 4.2 errors when conditions > 1L
Instead, removes NAs from `config`
Then, checks the length of `config`; if there is a length, adds it to
the TileDB config

fixes #518

* Update tests for input vectors and NAs

* Update changelog

[ci_skip]
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Safer check for NAs in tiledb_config() TileDB-Inc/TileDB-R
2 participants
@eddelbuettel @mojaveazure