paleolimbot commented on code in PR #288:
URL: https://github.com/apache/arrow-nanoarrow/pull/288#discussion_r1311651406
##########
r/R/convert-array.R:
##########
@@ -85,14 +85,28 @@ convert_array <- function(array, to = NULL, ...) {
#' @export
convert_array.default <- function(array, to = NULL, ..., .from_c = FALSE) {
if (.from_c) {
+ # Handle extension conversion
+ # We don't need the user-friendly versions and this is
performance-sensitive
+ schema <- .Call(nanoarrow_c_infer_schema_array, array)
+ parsed <- .Call(nanoarrow_c_schema_parse, schema)
+ if (!is.null(parsed$extension_name)) {
+ spec <- resolve_nanoarrow_extension(parsed$extension_name)
+ return(convert_array_extension(spec, array, to, ...))
+ }
+
# Handle default dictionary conversion since it's the same for all types
dictionary <- array$dictionary
if (!is.null(dictionary)) {
values <- .Call(nanoarrow_c_convert_array, dictionary, to)
array$dictionary <- NULL
indices <- .Call(nanoarrow_c_convert_array, array, integer())
- return(values[indices + 1L])
+
+ if (is.data.frame(values)) {
+ return(values[indices + 1L, , drop = FALSE])
+ } else {
+ return(values[indices + 1L])
+ }
Review Comment:
This also exposes a larger design question...currently, nanoarrow does not
require vctrs for most data types (it does require it for converting
lists-of-things into R vectors, and I think this is a good thing). I will
extract a utility function so that it is more obvious what this does/easier to
switch if we take on a vctrs dependency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
