From ae08cab9448c289fd94964cffe92fe0828e2327f Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Sun, 8 Oct 2023 18:18:42 -0700 Subject: [PATCH 01/17] New `add_pkg_name` to add ("arrow::") to code functions. Added private `code_name` to DataType object and refactored `code` accordingly. --- r/R/schema.R | 7 +-- r/R/type.R | 122 ++++++++++++++++++++++++++++++++++++--------------- r/R/util.R | 5 +++ 3 files changed, 95 insertions(+), 39 deletions(-) diff --git a/r/R/schema.R b/r/R/schema.R index ac0604b2b345c..6a48f280de7cc 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -107,14 +107,15 @@ Schema <- R6Class("Schema", inherits(other, "Schema") && Schema__Equals(self, other, isTRUE(check_metadata)) }, export_to_c = function(ptr) ExportSchema(self, ptr), - code = function() { + code = function(explicit_pkg_name=FALSE) { names <- self$names codes <- map2(names, self$fields, function(name, field) { - field$type$code() + field$type$code(explicit_pkg_name) }) codes <- set_names(codes, names) - call2("schema", !!!codes) + call_name <- add_pkg_name("schema", explicit_pkg_name) + call2(call_name, !!!codes) }, WithNames = function(names) { if (!inherits(names, "character")) { diff --git a/r/R/type.R b/r/R/type.R index 58d3267243220..778455c5ef44f 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -33,6 +33,7 @@ #' - `$id`: integer Arrow type id. #' - `$name`: string Arrow type name. #' - `$num_fields`: number of child fields. +#' - `$code_name`: Name of the call used by code(). #' #' @seealso [infer_type()] #' @rdname DataType-class @@ -51,12 +52,15 @@ DataType <- R6Class("DataType", DataType__fields(self) }, export_to_c = function(ptr) ExportType(self, ptr), - code = function() call("stop", paste0("Unsupported type: <", self$ToString(), ">.")) + code = function(explicit_pkg_name=FALSE) call(add_pkg_name(self$code_name(), explicit_pkg_name=explicit_pkg_name)) ), active = list( id = function() DataType__id(self), name = function() DataType__name(self), - num_fields = function() DataType__num_fields(self) + num_fields = function() DataType__num_fields(self), + ), + private = list( + code_name = function() call("stop", paste0("Unsupported type: <", self$ToString(), ">.")) ) ) @@ -157,11 +161,11 @@ infer_type.Expression <- function(x, ...) x$type() #' @name FixedWidthType FixedWidthType <- R6Class("FixedWidthType", inherit = DataType, - public = list( - code = function() call(tolower(self$name)) - ), active = list( bit_width = function() FixedWidthType__bit_width(self) + ), + private = list( + code_name = function() tolower(self$name) ) ) @@ -177,47 +181,60 @@ Float16 <- R6Class("Float16", inherit = FixedWidthType) Float32 <- R6Class("Float32", inherit = FixedWidthType) Float64 <- R6Class("Float64", inherit = FixedWidthType, - public = list( - code = function() call("float64") + private = list( + code_name = function() "float64" ) ) Boolean <- R6Class("Boolean", inherit = FixedWidthType) Utf8 <- R6Class("Utf8", inherit = DataType, - public = list( - code = function() call("utf8") + private = list( + code_name = function() "utf8" ) ) LargeUtf8 <- R6Class("LargeUtf8", inherit = DataType, - public = list( - code = function() call("large_utf8") + private = list( + code_name = function() "large_utf8" ) ) Binary <- R6Class("Binary", inherit = DataType, - public = list( - code = function() call("binary") + private = list( + code_name = function() "binary" ) ) LargeBinary <- R6Class("LargeBinary", - inherit = DataType, public = list( - code = function() call("large_binary") + inherit = DataType, + private = list( + code_name = function() "large_binary" ) ) FixedSizeBinary <- R6Class("FixedSizeBinary", inherit = FixedWidthType, public = list( byte_width = function() FixedSizeBinary__byte_width(self), - code = function() call2("fixed_size_binary", byte_width = self$byte_width()) + code = function(explicit_pkg_name=FALSE) { + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) + call2(call_name, byte_width = self$byte_width()) + } + ), + private = list( + code_name = function() "fixed_size_binary" ) ) DateType <- R6Class("DateType", inherit = FixedWidthType, public = list( - code = function() call2(tolower(self$name)), + code = function(explicit_pkg_name=FALSE) { + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) + call2(call_name) + }, unit = function() DateType__unit(self) + ), + private = list( + code_name = function() tolower(self$name) ) ) Date32 <- R6Class("Date32", inherit = DateType) @@ -232,27 +249,35 @@ TimeType <- R6Class("TimeType", Time32 <- R6Class("Time32", inherit = TimeType, public = list( - code = function() { + code = function(explicit_pkg_name=FALSE) { unit <- if (self$unit() == TimeUnit$MILLI) { "ms" } else { "s" } - call2("time32", unit = unit) + call_name <- add_pkg_name(self$code_name, explicit_pkg_name) + call2(call_name, unit = unit) } + ), + private = list( + code_name = function() "time32" ) ) Time64 <- R6Class("Time64", inherit = TimeType, public = list( - code = function() { + code = function(explicit_pkg_name=FALSE) { unit <- if (self$unit() == TimeUnit$NANO) { "ns" } else { "us" } - call2("time64", unit = unit) + call_name <- add_pkg_name(self$code_name, explicit_pkg_name) + call2(call_name, unit = unit) } + ), + private = list( + code_name = function() "time64" ) ) @@ -265,36 +290,45 @@ DurationType <- R6Class("DurationType", Null <- R6Class("Null", inherit = DataType, - public = list( - code = function() call("null") + private = list( + code_name = function() "null" ) ) Timestamp <- R6Class("Timestamp", inherit = FixedWidthType, public = list( - code = function() { + code = function(explicit_pkg_name=FALSE) { unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] tz <- self$timezone() + + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) if (identical(tz, "")) { - call2("timestamp", unit = unit) + call2(call_name, unit = unit) } else { - call2("timestamp", unit = unit, timezone = tz) + call2(call_name, unit = unit, timezone = tz) } }, timezone = function() TimestampType__timezone(self), unit = function() TimestampType__unit(self) + ), + private = list( + code_name = function() "timestamp" ) ) DecimalType <- R6Class("DecimalType", inherit = FixedWidthType, public = list( - code = function() { - call2("decimal", precision = self$precision(), scale = self$scale()) + code = function(explicit_pkg_name=FALSE) { + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) + call2(call_name, precision = self$precision(), scale = self$scale()) }, precision = function() DecimalType__precision(self), scale = function() DecimalType__scale(self) + ), + private = list( + code_name = function() "decimal" ) ) @@ -624,16 +658,20 @@ check_decimal_args <- function(precision, scale) { StructType <- R6Class("StructType", inherit = NestedType, public = list( - code = function() { + code = function(explicit_pkg_name=FALSE) { field_names <- StructType__field_names(self) codes <- map(field_names, function(name) { self$GetFieldByName(name)$type$code() }) codes <- set_names(codes, field_names) - call2("struct", !!!codes) + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) + call2(call_name, !!!codes) }, GetFieldByName = function(name) StructType__GetFieldByName(self, name), GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) + ), + private = list( + code_name = function() "struct" ) ) StructType$create <- function(...) struct__(.fields(list(...))) @@ -648,13 +686,17 @@ names.StructType <- function(x) StructType__field_names(x) ListType <- R6Class("ListType", inherit = NestedType, public = list( - code = function() { - call("list_of", self$value_type$code()) + code = function(explicit_pkg_name=FALSE) { + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) + call(call_name, self$value_type$code()) } ), active = list( value_field = function() ListType__value_field(self), value_type = function() ListType__value_type(self) + ), + private = list( + code_name = function() "list_of" ) ) @@ -665,13 +707,17 @@ list_of <- function(type) list__(type) LargeListType <- R6Class("LargeListType", inherit = NestedType, public = list( - code = function() { - call2("large_list_of", self$value_type$code()) + code = function(explicit_pkg_name=FALSE) { + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) + call2(call_name, self$value_type$code()) } ), active = list( value_field = function() LargeListType__value_field(self), value_type = function() LargeListType__value_type(self) + ), + private = list( + code_name = function() "large_list_of" ) ) @@ -684,14 +730,18 @@ large_list_of <- function(type) large_list__(type) FixedSizeListType <- R6Class("FixedSizeListType", inherit = NestedType, public = list( - code = function() { - call2("fixed_size_list_of", self$value_type$code(), list_size = self$list_size) + code = function(explicit_pkg_name=FALSE) { + call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) + call2(call_name, self$value_type$code(), list_size = self$list_size) } ), active = list( value_field = function() FixedSizeListType__value_field(self), value_type = function() FixedSizeListType__value_type(self), list_size = function() FixedSizeListType__list_size(self) + ), + private = list( + code_name = function() "fixed_size_list_of" ) ) diff --git a/r/R/util.R b/r/R/util.R index a7cb5b3792d29..b6890de74b41a 100644 --- a/r/R/util.R +++ b/r/R/util.R @@ -262,3 +262,8 @@ check_named_cols <- function(df) { ) } } + +add_pkg_name <- function(x, explicit_pkg_name) { + if(!explicit_pkg_name) x + else paste0("arrow::", x) +} From 92996a0f9fb5cad92cd9626bd3d984c4e04d5603 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Sun, 8 Oct 2023 19:06:36 -0700 Subject: [PATCH 02/17] Update the DataType roxygen docstring with explicit_pkg_name. Add `code` to the Schema docstring. --- r/R/schema.R | 1 + r/R/type.R | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/r/R/schema.R b/r/R/schema.R index 6a48f280de7cc..a58068bc46213 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -39,6 +39,7 @@ #' - `$WithMetadata(metadata)`: returns a new `Schema` with the key-value #' `metadata` set. Note that all list elements in `metadata` will be coerced #' to `character`. +#' - `$code(explicit_pkg_name)`: Produces an R call for the schema. Use `explicit_pkg_name=TRUE` to call with `arrow::`. #' #' @section Active bindings: #' diff --git a/r/R/type.R b/r/R/type.R index 778455c5ef44f..e54ae846e0982 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -27,7 +27,7 @@ #' - `$ToString()`: String representation of the DataType #' - `$Equals(other)`: Is the DataType equal to `other` #' - `$fields()`: The children fields associated with this type -#' - `$code()`: Produces an R call of the data type. +#' - `$code(explicit_pkg_name)`: Produces an R call of the data type. Use `explicit_pkg_name=TRUE` to call with `arrow::`. #' #' There are also some active bindings: #' - `$id`: integer Arrow type id. From f8a9520fb8ad550202e2ad5e24c83eecc72c4ca6 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Sun, 8 Oct 2023 19:10:18 -0700 Subject: [PATCH 03/17] Fix -- dangling comma. --- r/R/type.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/type.R b/r/R/type.R index e54ae846e0982..6ba525c8180ca 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -57,7 +57,7 @@ DataType <- R6Class("DataType", active = list( id = function() DataType__id(self), name = function() DataType__name(self), - num_fields = function() DataType__num_fields(self), + num_fields = function() DataType__num_fields(self) ), private = list( code_name = function() call("stop", paste0("Unsupported type: <", self$ToString(), ">.")) From d5718bd038e1237edd407ef5448d13b46ab61c09 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Sun, 8 Oct 2023 19:12:18 -0700 Subject: [PATCH 04/17] Style --- r/R/type.R | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/r/R/type.R b/r/R/type.R index 6ba525c8180ca..d6af79af88340 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -52,7 +52,7 @@ DataType <- R6Class("DataType", DataType__fields(self) }, export_to_c = function(ptr) ExportType(self, ptr), - code = function(explicit_pkg_name=FALSE) call(add_pkg_name(self$code_name(), explicit_pkg_name=explicit_pkg_name)) + code = function(explicit_pkg_name = FALSE) call(add_pkg_name(self$code_name(), explicit_pkg_name = explicit_pkg_name)) ), active = list( id = function() DataType__id(self), @@ -214,7 +214,7 @@ FixedSizeBinary <- R6Class("FixedSizeBinary", inherit = FixedWidthType, public = list( byte_width = function() FixedSizeBinary__byte_width(self), - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) call2(call_name, byte_width = self$byte_width()) } @@ -227,7 +227,7 @@ FixedSizeBinary <- R6Class("FixedSizeBinary", DateType <- R6Class("DateType", inherit = FixedWidthType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) call2(call_name) }, @@ -249,7 +249,7 @@ TimeType <- R6Class("TimeType", Time32 <- R6Class("Time32", inherit = TimeType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { unit <- if (self$unit() == TimeUnit$MILLI) { "ms" } else { @@ -266,7 +266,7 @@ Time32 <- R6Class("Time32", Time64 <- R6Class("Time64", inherit = TimeType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { unit <- if (self$unit() == TimeUnit$NANO) { "ns" } else { @@ -298,7 +298,7 @@ Null <- R6Class("Null", Timestamp <- R6Class("Timestamp", inherit = FixedWidthType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] tz <- self$timezone() @@ -320,7 +320,7 @@ Timestamp <- R6Class("Timestamp", DecimalType <- R6Class("DecimalType", inherit = FixedWidthType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) call2(call_name, precision = self$precision(), scale = self$scale()) }, @@ -658,7 +658,7 @@ check_decimal_args <- function(precision, scale) { StructType <- R6Class("StructType", inherit = NestedType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { field_names <- StructType__field_names(self) codes <- map(field_names, function(name) { self$GetFieldByName(name)$type$code() @@ -686,7 +686,7 @@ names.StructType <- function(x) StructType__field_names(x) ListType <- R6Class("ListType", inherit = NestedType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) call(call_name, self$value_type$code()) } @@ -707,7 +707,7 @@ list_of <- function(type) list__(type) LargeListType <- R6Class("LargeListType", inherit = NestedType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) call2(call_name, self$value_type$code()) } @@ -730,7 +730,7 @@ large_list_of <- function(type) large_list__(type) FixedSizeListType <- R6Class("FixedSizeListType", inherit = NestedType, public = list( - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) call2(call_name, self$value_type$code(), list_size = self$list_size) } From e19868d4360a41c158cf159877f42f1f06e6ca20 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Sun, 8 Oct 2023 19:13:34 -0700 Subject: [PATCH 05/17] Style --- r/R/schema.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/schema.R b/r/R/schema.R index a58068bc46213..ee28dd65b0543 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -108,7 +108,7 @@ Schema <- R6Class("Schema", inherits(other, "Schema") && Schema__Equals(self, other, isTRUE(check_metadata)) }, export_to_c = function(ptr) ExportSchema(self, ptr), - code = function(explicit_pkg_name=FALSE) { + code = function(explicit_pkg_name = FALSE) { names <- self$names codes <- map2(names, self$fields, function(name, field) { field$type$code(explicit_pkg_name) From 6d587baab4074f3694d06ef96907e94c2ec2653c Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Wed, 11 Oct 2023 20:10:18 -0700 Subject: [PATCH 06/17] Fix implementation -- now uses call2(.ns=...) to build call with namespace. --- r/R/schema.R | 9 ++--- r/R/type.R | 73 ++++++++++++++++------------------------- r/R/util.R | 8 +++-- r/man/DataType-class.Rd | 2 +- r/man/Schema-class.Rd | 1 + r/man/get_pkg_ns.Rd | 14 ++++++++ 6 files changed, 55 insertions(+), 52 deletions(-) create mode 100644 r/man/get_pkg_ns.Rd diff --git a/r/R/schema.R b/r/R/schema.R index ee28dd65b0543..cec1a4e7da538 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -111,12 +111,10 @@ Schema <- R6Class("Schema", code = function(explicit_pkg_name = FALSE) { names <- self$names codes <- map2(names, self$fields, function(name, field) { - field$type$code(explicit_pkg_name) + field$type$code(explicit_pkg_name = explicit_pkg_name) }) codes <- set_names(codes, names) - - call_name <- add_pkg_name("schema", explicit_pkg_name) - call2(call_name, !!!codes) + call2(private$call_name(), !!!codes, .ns = get_pkg_ns(explicit_pkg_name)) }, WithNames = function(names) { if (!inherits(names, "character")) { @@ -184,6 +182,9 @@ Schema <- R6Class("Schema", self } } + ), + private = list( + call_name = function() "schema" ) ) Schema$create <- function(...) { diff --git a/r/R/type.R b/r/R/type.R index d6af79af88340..53a93a6d2d3f4 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -33,7 +33,6 @@ #' - `$id`: integer Arrow type id. #' - `$name`: string Arrow type name. #' - `$num_fields`: number of child fields. -#' - `$code_name`: Name of the call used by code(). #' #' @seealso [infer_type()] #' @rdname DataType-class @@ -52,7 +51,7 @@ DataType <- R6Class("DataType", DataType__fields(self) }, export_to_c = function(ptr) ExportType(self, ptr), - code = function(explicit_pkg_name = FALSE) call(add_pkg_name(self$code_name(), explicit_pkg_name = explicit_pkg_name)) + code = function(explicit_pkg_name=FALSE) call2(private$call_name(), .ns=get_pkg_ns(explicit_pkg_name)) ), active = list( id = function() DataType__id(self), @@ -60,7 +59,7 @@ DataType <- R6Class("DataType", num_fields = function() DataType__num_fields(self) ), private = list( - code_name = function() call("stop", paste0("Unsupported type: <", self$ToString(), ">.")) + call_name = function() call("stop", paste0("Unsupported type: <", self$ToString(), ">.")) ) ) @@ -165,7 +164,7 @@ FixedWidthType <- R6Class("FixedWidthType", bit_width = function() FixedWidthType__bit_width(self) ), private = list( - code_name = function() tolower(self$name) + call_name = function() tolower(self$name) ) ) @@ -182,32 +181,32 @@ Float32 <- R6Class("Float32", inherit = FixedWidthType) Float64 <- R6Class("Float64", inherit = FixedWidthType, private = list( - code_name = function() "float64" + call_name = function() "float64" ) ) Boolean <- R6Class("Boolean", inherit = FixedWidthType) Utf8 <- R6Class("Utf8", inherit = DataType, private = list( - code_name = function() "utf8" + call_name = function() "utf8" ) ) LargeUtf8 <- R6Class("LargeUtf8", inherit = DataType, private = list( - code_name = function() "large_utf8" + call_name = function() "large_utf8" ) ) Binary <- R6Class("Binary", inherit = DataType, private = list( - code_name = function() "binary" + call_name = function() "binary" ) ) LargeBinary <- R6Class("LargeBinary", inherit = DataType, private = list( - code_name = function() "large_binary" + call_name = function() "large_binary" ) ) FixedSizeBinary <- R6Class("FixedSizeBinary", @@ -215,26 +214,21 @@ FixedSizeBinary <- R6Class("FixedSizeBinary", public = list( byte_width = function() FixedSizeBinary__byte_width(self), code = function(explicit_pkg_name = FALSE) { - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) - call2(call_name, byte_width = self$byte_width()) + call2(private$call_name(), byte_width = self$byte_width(), .ns=get_pkg_ns(explicit_pkg_name)) } ), private = list( - code_name = function() "fixed_size_binary" + call_name = function() "fixed_size_binary" ) ) DateType <- R6Class("DateType", inherit = FixedWidthType, public = list( - code = function(explicit_pkg_name = FALSE) { - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) - call2(call_name) - }, unit = function() DateType__unit(self) ), private = list( - code_name = function() tolower(self$name) + call_name = function() tolower(self$name) ) ) Date32 <- R6Class("Date32", inherit = DateType) @@ -255,12 +249,11 @@ Time32 <- R6Class("Time32", } else { "s" } - call_name <- add_pkg_name(self$code_name, explicit_pkg_name) - call2(call_name, unit = unit) + call2(private$call_name, unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) } ), private = list( - code_name = function() "time32" + call_name = function() "time32" ) ) Time64 <- R6Class("Time64", @@ -272,12 +265,11 @@ Time64 <- R6Class("Time64", } else { "us" } - call_name <- add_pkg_name(self$code_name, explicit_pkg_name) - call2(call_name, unit = unit) + call2(private$call_name, unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) } ), private = list( - code_name = function() "time64" + call_name = function() "time64" ) ) @@ -291,7 +283,7 @@ DurationType <- R6Class("DurationType", Null <- R6Class("Null", inherit = DataType, private = list( - code_name = function() "null" + call_name = function() "null" ) ) @@ -301,19 +293,17 @@ Timestamp <- R6Class("Timestamp", code = function(explicit_pkg_name = FALSE) { unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] tz <- self$timezone() - - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) if (identical(tz, "")) { - call2(call_name, unit = unit) + call2(private$call_name, unit = unit, .ns = get_pkg_ns(explicit_pkg_name)) } else { - call2(call_name, unit = unit, timezone = tz) + call2(private$call_name, unit = unit, timezone = tz, .ns = get_pkg_ns(explicit_pkg_name)) } }, timezone = function() TimestampType__timezone(self), unit = function() TimestampType__unit(self) ), private = list( - code_name = function() "timestamp" + call_name = function() "timestamp" ) ) @@ -321,14 +311,13 @@ DecimalType <- R6Class("DecimalType", inherit = FixedWidthType, public = list( code = function(explicit_pkg_name = FALSE) { - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) - call2(call_name, precision = self$precision(), scale = self$scale()) + call2(private$call_name, precision = self$precision(), scale = self$scale(), .ns = get_pkg_ns(explicit_pkg_name)) }, precision = function() DecimalType__precision(self), scale = function() DecimalType__scale(self) ), private = list( - code_name = function() "decimal" + call_name = function() "decimal" ) ) @@ -664,14 +653,13 @@ StructType <- R6Class("StructType", self$GetFieldByName(name)$type$code() }) codes <- set_names(codes, field_names) - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) - call2(call_name, !!!codes) + call2(private$call_name, !!!codes, .ns = get_pkg_ns(explicit_pkg_name)) }, GetFieldByName = function(name) StructType__GetFieldByName(self, name), GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) ), private = list( - code_name = function() "struct" + call_name = function() "struct" ) ) StructType$create <- function(...) struct__(.fields(list(...))) @@ -687,8 +675,7 @@ ListType <- R6Class("ListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) - call(call_name, self$value_type$code()) + call2(private$call_name, self$value_type$code(), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( @@ -696,7 +683,7 @@ ListType <- R6Class("ListType", value_type = function() ListType__value_type(self) ), private = list( - code_name = function() "list_of" + call_name = function() "list_of" ) ) @@ -708,8 +695,7 @@ LargeListType <- R6Class("LargeListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) - call2(call_name, self$value_type$code()) + call2(private$call_name, self$value_type$code(), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( @@ -717,7 +703,7 @@ LargeListType <- R6Class("LargeListType", value_type = function() LargeListType__value_type(self) ), private = list( - code_name = function() "large_list_of" + call_name = function() "large_list_of" ) ) @@ -731,8 +717,7 @@ FixedSizeListType <- R6Class("FixedSizeListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call_name <- add_pkg_name(self$code_name(), explicit_pkg_name) - call2(call_name, self$value_type$code(), list_size = self$list_size) + call2(private$call_name, self$value_type$code(), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( @@ -741,7 +726,7 @@ FixedSizeListType <- R6Class("FixedSizeListType", list_size = function() FixedSizeListType__list_size(self) ), private = list( - code_name = function() "fixed_size_list_of" + call_name = function() "fixed_size_list_of" ) ) diff --git a/r/R/util.R b/r/R/util.R index b6890de74b41a..7b49537164ffa 100644 --- a/r/R/util.R +++ b/r/R/util.R @@ -263,7 +263,9 @@ check_named_cols <- function(df) { } } -add_pkg_name <- function(x, explicit_pkg_name) { - if(!explicit_pkg_name) x - else paste0("arrow::", x) +#' Prepares the value of `.ns` to use with `rlang::call2()` +#' +#' @param explicit_pkg_name Whether to include the package name or not. +get_pkg_ns <- function(explicit_pkg_name) { + if(explicit_pkg_name) getPackageName() else NULL } diff --git a/r/man/DataType-class.Rd b/r/man/DataType-class.Rd index 4f95578133bd1..1c00422b3e4f0 100644 --- a/r/man/DataType-class.Rd +++ b/r/man/DataType-class.Rd @@ -13,7 +13,7 @@ DataType class \item \verb{$ToString()}: String representation of the DataType \item \verb{$Equals(other)}: Is the DataType equal to \code{other} \item \verb{$fields()}: The children fields associated with this type -\item \verb{$code()}: Produces an R call of the data type. +\item \verb{$code(explicit_pkg_name)}: Produces an R call of the data type. Use \code{explicit_pkg_name=TRUE} to call with \verb{arrow::}. } There are also some active bindings: diff --git a/r/man/Schema-class.Rd b/r/man/Schema-class.Rd index 32250cdfe7d05..a85e2e11edce1 100644 --- a/r/man/Schema-class.Rd +++ b/r/man/Schema-class.Rd @@ -24,6 +24,7 @@ Many Arrow objects, including \link{Table} and \link{Dataset}, have a \verb{$sch \item \verb{$WithMetadata(metadata)}: returns a new \code{Schema} with the key-value \code{metadata} set. Note that all list elements in \code{metadata} will be coerced to \code{character}. +\item \verb{$code(explicit_pkg_name)}: Produces an R call for the schema. Use \code{explicit_pkg_name=TRUE} to call with \verb{arrow::}. } } diff --git a/r/man/get_pkg_ns.Rd b/r/man/get_pkg_ns.Rd new file mode 100644 index 0000000000000..b018e6f15623a --- /dev/null +++ b/r/man/get_pkg_ns.Rd @@ -0,0 +1,14 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/util.R +\name{get_pkg_ns} +\alias{get_pkg_ns} +\title{Prepares the value of \code{.ns} to use with \code{rlang::call2()}} +\usage{ +get_pkg_ns(explicit_pkg_name) +} +\arguments{ +\item{explicit_pkg_name}{Whether to include the package name or not.} +} +\description{ +Prepares the value of \code{.ns} to use with \code{rlang::call2()} +} From cc37996d61c3791d09f92907ebfe3908f7409d9e Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Wed, 11 Oct 2023 20:20:26 -0700 Subject: [PATCH 07/17] Pass `explicit_pkg_name` to all recursive calls of `$code()` in `NestedType`s. --- r/R/type.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/r/R/type.R b/r/R/type.R index 53a93a6d2d3f4..451f4bc30e67f 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -650,7 +650,7 @@ StructType <- R6Class("StructType", code = function(explicit_pkg_name = FALSE) { field_names <- StructType__field_names(self) codes <- map(field_names, function(name) { - self$GetFieldByName(name)$type$code() + self$GetFieldByName(name)$type$code(explicit_pkg_name) }) codes <- set_names(codes, field_names) call2(private$call_name, !!!codes, .ns = get_pkg_ns(explicit_pkg_name)) @@ -675,7 +675,7 @@ ListType <- R6Class("ListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call2(private$call_name, self$value_type$code(), .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name, self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( @@ -695,7 +695,7 @@ LargeListType <- R6Class("LargeListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call2(private$call_name, self$value_type$code(), .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name, self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( @@ -717,7 +717,7 @@ FixedSizeListType <- R6Class("FixedSizeListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call2(private$call_name, self$value_type$code(), .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name, self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( From 959dae46364bcb89e40ef39223bb4100525bf39a Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Thu, 12 Oct 2023 00:09:07 -0700 Subject: [PATCH 08/17] Fix errors in `code()`. Added tests for all types. Added test for schema code. Added `explicit_pkg_name` to `expect_code_roundtrip()`. --- r/R/type.R | 18 ++-- r/tests/testthat/helper-roundtrip.R | 4 +- r/tests/testthat/test-schema.R | 6 +- r/tests/testthat/test-type.R | 157 ++++++++++++++++++++++++++++ 4 files changed, 171 insertions(+), 14 deletions(-) diff --git a/r/R/type.R b/r/R/type.R index 451f4bc30e67f..caed332eaa54c 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -249,7 +249,7 @@ Time32 <- R6Class("Time32", } else { "s" } - call2(private$call_name, unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) } ), private = list( @@ -265,7 +265,7 @@ Time64 <- R6Class("Time64", } else { "us" } - call2(private$call_name, unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) } ), private = list( @@ -294,9 +294,9 @@ Timestamp <- R6Class("Timestamp", unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] tz <- self$timezone() if (identical(tz, "")) { - call2(private$call_name, unit = unit, .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), unit = unit, .ns = get_pkg_ns(explicit_pkg_name)) } else { - call2(private$call_name, unit = unit, timezone = tz, .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), unit = unit, timezone = tz, .ns = get_pkg_ns(explicit_pkg_name)) } }, timezone = function() TimestampType__timezone(self), @@ -311,7 +311,7 @@ DecimalType <- R6Class("DecimalType", inherit = FixedWidthType, public = list( code = function(explicit_pkg_name = FALSE) { - call2(private$call_name, precision = self$precision(), scale = self$scale(), .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), precision = self$precision(), scale = self$scale(), .ns = get_pkg_ns(explicit_pkg_name)) }, precision = function() DecimalType__precision(self), scale = function() DecimalType__scale(self) @@ -653,7 +653,7 @@ StructType <- R6Class("StructType", self$GetFieldByName(name)$type$code(explicit_pkg_name) }) codes <- set_names(codes, field_names) - call2(private$call_name, !!!codes, .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), !!!codes, .ns = get_pkg_ns(explicit_pkg_name)) }, GetFieldByName = function(name) StructType__GetFieldByName(self, name), GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) @@ -675,7 +675,7 @@ ListType <- R6Class("ListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call2(private$call_name, self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( @@ -695,7 +695,7 @@ LargeListType <- R6Class("LargeListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call2(private$call_name, self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( @@ -717,7 +717,7 @@ FixedSizeListType <- R6Class("FixedSizeListType", inherit = NestedType, public = list( code = function(explicit_pkg_name = FALSE) { - call2(private$call_name, self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) + call2(private$call_name(), self$value_type$code(explicit_pkg_name), list_size = self$list_size, .ns = get_pkg_ns(explicit_pkg_name)) } ), active = list( diff --git a/r/tests/testthat/helper-roundtrip.R b/r/tests/testthat/helper-roundtrip.R index d6b965ca6597d..57132542efb6d 100644 --- a/r/tests/testthat/helper-roundtrip.R +++ b/r/tests/testthat/helper-roundtrip.R @@ -43,6 +43,6 @@ expect_array_roundtrip <- function(x, type, as = NULL) { invisible(a) } -expect_code_roundtrip <- function(x) { - expect_equal(eval(x$code()), x) +expect_code_roundtrip <- function(x, explicit_pkg_name = FALSE, ...) { + expect_equal(eval(x$code(explicit_pkg_name)), x, ...) } diff --git a/r/tests/testthat/test-schema.R b/r/tests/testthat/test-schema.R index 15342add38fae..5df0573f6443f 100644 --- a/r/tests/testthat/test-schema.R +++ b/r/tests/testthat/test-schema.R @@ -39,9 +39,9 @@ test_that("Schema print method", { }) test_that("Schema$code()", { - expect_code_roundtrip( - schema(a = int32(), b = struct(c = double(), d = utf8()), e = list_of(binary())) - ) + schema_obj <- schema(a = int32(), b = struct(c = double(), d = utf8()), e = list_of(binary())) + expect_code_roundtrip(schema_obj) + expect_code_roundtrip(schema_obj, explicit_pkg_name = TRUE) skip_if(packageVersion("rlang") < "1") expect_error( diff --git a/r/tests/testthat/test-type.R b/r/tests/testthat/test-type.R index 4f6210c29c1a2..f1183c7eba751 100644 --- a/r/tests/testthat/test-type.R +++ b/r/tests/testthat/test-type.R @@ -343,3 +343,160 @@ test_that("infer_type() infers type for lists starting with NULL - ARROW-17639", list_of(null()) ) }) + + +test_that("code() works for data types without arguments",{ + # Names encode type aliases. + # No names means the type alias matches the string + type_strs <- c( + "int8", "int16", "int32", "int64", + "uint8", "uint16", "uint32", "uint64", + "halffloat" = "float16", "halffloat", "float" = "float32", "float", "float64", + "bool" = "boolean", "bool", + "utf8", "large_utf8", "binary", "large_binary", "utf8" = "string", + "null", + "date32", "date64", "time32", "time64", "timestamp" + ) + + evaluate_type_str <- function(type_str, type_alias) { + if(type_alias == "") { + type_alias <- type_str + } + type_obj <- eval(call2(type_str, .ns=getPackageName())) + + expect_code_roundtrip(type_obj, info = type_str) + expect_code_roundtrip(type_obj, explicit_pkg_name = TRUE, info = type_str) + + type_code <- as.character(type_obj$code())[1] # Ignore units in time types. + type_code_with_ns <- as.character(type_obj$code(TRUE))[1] # Ignore units in time types. + + expect_equal(type_code, type_alias) + expect_equal(type_code_with_ns, paste0(getPackageName(),"::", type_alias)) + } + purrr::iwalk(type_strs, evaluate_type_str) + +}) + +test_that("code() works for simple data types with arguments",{ + types_with_args <- list( + # type_str, args, type_alias + list("fixed_size_binary", list(42), "fixed_size_binary"), + list("decimal", list(3, 2), "decimal"), + list("decimal128", list(3, 2), "decimal"), + list("decimal256", list(3, 2), "decimal") + ) + evaluate_type_with_arg <- function(type_with_args) { + type_str <- type_with_args[[1]] + args <- type_with_args[[2]] + type_alias <- type_with_args[[3]] + + type_obj <- eval(call2(type_str, !!!args, .ns=getPackageName())) + + if(type_str == type_alias) { + expect_code_roundtrip(type_obj, info = type_str) + expect_code_roundtrip(type_obj, explicit_pkg_name = TRUE, info = type_str) + } + + type_code <- as.character(type_obj$code()) + type_code_with_ns <- as.character(type_obj$code(TRUE)) + + # test info + build_test_info <- function(test_str, explicit_pkg_name=FALSE) { + glue::glue("`{type_str}` {test_str} (explicit_pkg_name={explicit_pkg_name})") + } + + # type name + expect_equal(type_code[1], type_alias, info=build_test_info("type name")) + expect_equal(type_code_with_ns[1], paste0(getPackageName(), "::", type_alias), info=build_test_info("type name", TRUE)) + + # args + expect_equal(type_code[-1], as.character(unlist(args)), info=build_test_info("args")) + expect_equal(type_code_with_ns[-1], as.character(unlist(args)), info=build_test_info("type name", TRUE)) + } + purrr::walk(types_with_args, evaluate_type_with_arg) +}) + +test_that("code() works for nested_types",{ + # Nested Types + nested_types <- list( + list("struct", list(foo=int32())), + list("list_of", list(int32())), + list("large_list_of", list(int32())), + list("fixed_size_list_of", list(int32(), 42)) + ) + evaluate_nested_type <- function(nested_type) { + type_str <- nested_type[[1]] + args <- nested_type[[2]] + + type_obj <- eval(call2(type_str, !!!args, .ns=getPackageName())) + + expect_code_roundtrip(type_obj, info = type_str) + expect_code_roundtrip(type_obj, explicit_pkg_name = TRUE, info = type_str) + + type_code <- as.character(type_obj$code()) + type_code_with_ns <- as.character(type_obj$code(TRUE)) + + + # test info + build_test_info <- function(test_str, explicit_pkg_name=FALSE) { + glue::glue("`{type_str}` {test_str} (explicit_pkg_name={explicit_pkg_name})") + } + + # type name + expect_equal(type_code[1], type_str, info=build_test_info("type name")) + expect_equal(type_code_with_ns[1], paste0(getPackageName(), "::", type_str), + info=build_test_info("type name", TRUE)) + + # first arg (also a type) + build_expected_str <- function(explicit_pkg_name=FALSE) { + paste0( + as.character(args[[1]]$code(explicit_pkg_name)), + "()" # The () is kept in arguments. + ) + } + expect_equal(type_code[2], build_expected_str(), info=build_test_info("first arg")) + expect_equal(type_code_with_ns[2], build_expected_str(TRUE), info=build_test_info("first arg", TRUE)) + + # second arg, if exists + if(length(args) == 2) { + second_arg <- as.character(args[[2]]) + expect_equal(type_code[3], second_arg, info=build_test_info("second arg")) + expect_equal(type_code_with_ns[3], second_arg, info=build_test_info("second arg", TRUE)) + } + } + purrr::walk(nested_types, evaluate_nested_type) + +}) + +test_that("code() works for map_of",{ + + type_str <- "map_of" + args <- list(string(), string()) + type_obj <- eval(call2(type_str, !!!args, .ns=getPackageName())) + type_code <- as.character(type_obj$code()) + type_code_with_ns <- as.character(type_obj$code(TRUE)) + + + # test info + build_test_info <- function(test_str, explicit_pkg_name=FALSE) { + glue::glue("`{type_str}` {test_str} (explicit_pkg_name={explicit_pkg_name})") + } + + # list_of + expect_equal(type_code[1], "list_of", info=build_test_info("list_of")) + expect_equal(type_code_with_ns[1], paste0(getPackageName(), "::", "list_of"), + info=build_test_info("list_of", TRUE)) + + # struct argument + expect_struct_code_matches <- function(struct_code, explicit_pkg_name=FALSE) { + get_code_str <- function(obj) as.character(obj$code(explicit_pkg_name)) + + expect_true(grepl(get_code_str(arrow::struct()), struct_code, fixed = TRUE)) + for(arg in args) { + expect_true(grepl(get_code_str(arg), struct_code, fixed = TRUE)) + } + } + + expect_struct_code_matches(type_code[2]) + expect_struct_code_matches(type_code_with_ns[2]) +}) From e0c655f2ece476bf11d43baedf7a8bc66e7bc0b3 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Thu, 12 Oct 2023 22:11:48 -0700 Subject: [PATCH 09/17] Apply reviewer recommendations: revert private$call_name, use if(namespace) "arrow". --- r/R/schema.R | 11 +- r/R/type.R | 445 +++++++++++++--------------- r/R/util.R | 7 - r/man/DataType-class.Rd | 2 +- r/man/Schema-class.Rd | 2 +- r/man/get_pkg_ns.Rd | 14 - r/tests/testthat/helper-roundtrip.R | 4 +- r/tests/testthat/test-schema.R | 10 +- r/tests/testthat/test-type.R | 34 +-- 9 files changed, 239 insertions(+), 290 deletions(-) delete mode 100644 r/man/get_pkg_ns.Rd diff --git a/r/R/schema.R b/r/R/schema.R index cec1a4e7da538..f95af9fd2c195 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -39,7 +39,7 @@ #' - `$WithMetadata(metadata)`: returns a new `Schema` with the key-value #' `metadata` set. Note that all list elements in `metadata` will be coerced #' to `character`. -#' - `$code(explicit_pkg_name)`: Produces an R call for the schema. Use `explicit_pkg_name=TRUE` to call with `arrow::`. +#' - `$code(namespace)`: Produces an R call for the schema. Use `namespace=TRUE` to call with `arrow::`. #' #' @section Active bindings: #' @@ -108,13 +108,13 @@ Schema <- R6Class("Schema", inherits(other, "Schema") && Schema__Equals(self, other, isTRUE(check_metadata)) }, export_to_c = function(ptr) ExportSchema(self, ptr), - code = function(explicit_pkg_name = FALSE) { + code = function(namespace = FALSE) { names <- self$names codes <- map2(names, self$fields, function(name, field) { - field$type$code(explicit_pkg_name = explicit_pkg_name) + field$type$code(namespace) }) codes <- set_names(codes, names) - call2(private$call_name(), !!!codes, .ns = get_pkg_ns(explicit_pkg_name)) + call2("schema", !!!codes, .ns = if(namespace) "arrow") }, WithNames = function(names) { if (!inherits(names, "character")) { @@ -182,9 +182,6 @@ Schema <- R6Class("Schema", self } } - ), - private = list( - call_name = function() "schema" ) ) Schema$create <- function(...) { diff --git a/r/R/type.R b/r/R/type.R index caed332eaa54c..0bfc8c20d1ea1 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -27,7 +27,7 @@ #' - `$ToString()`: String representation of the DataType #' - `$Equals(other)`: Is the DataType equal to `other` #' - `$fields()`: The children fields associated with this type -#' - `$code(explicit_pkg_name)`: Produces an R call of the data type. Use `explicit_pkg_name=TRUE` to call with `arrow::`. +#' - `$code(namespace)`: Produces an R call for the schema. Use `namespace=TRUE` to call with `arrow::`. #' #' There are also some active bindings: #' - `$id`: integer Arrow type id. @@ -39,28 +39,25 @@ #' @name DataType #' @seealso [`data-type`] DataType <- R6Class("DataType", - inherit = ArrowObject, - public = list( - ToString = function() { - DataType__ToString(self) - }, - Equals = function(other, check_metadata = FALSE, ...) { - inherits(other, "DataType") && DataType__Equals(self, other, isTRUE(check_metadata)) - }, - fields = function() { - DataType__fields(self) - }, - export_to_c = function(ptr) ExportType(self, ptr), - code = function(explicit_pkg_name=FALSE) call2(private$call_name(), .ns=get_pkg_ns(explicit_pkg_name)) - ), - active = list( - id = function() DataType__id(self), - name = function() DataType__name(self), - num_fields = function() DataType__num_fields(self) - ), - private = list( - call_name = function() call("stop", paste0("Unsupported type: <", self$ToString(), ">.")) - ) + inherit = ArrowObject, + public = list( + ToString = function() { + DataType__ToString(self) + }, + Equals = function(other, check_metadata = FALSE, ...) { + inherits(other, "DataType") && DataType__Equals(self, other, isTRUE(check_metadata)) + }, + fields = function() { + DataType__fields(self) + }, + export_to_c = function(ptr) ExportType(self, ptr), + code = function(namespace = FALSE) call2("stop", paste0("Unsupported type: <", self$ToString(), ">.")) + ), + active = list( + id = function() DataType__id(self), + name = function() DataType__name(self), + num_fields = function() DataType__num_fields(self) + ) ) #' @include arrowExports.R @@ -159,13 +156,13 @@ infer_type.Expression <- function(x, ...) x$type() #' @rdname FixedWidthType #' @name FixedWidthType FixedWidthType <- R6Class("FixedWidthType", - inherit = DataType, - active = list( - bit_width = function() FixedWidthType__bit_width(self) - ), - private = list( - call_name = function() tolower(self$name) - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow") + ), + active = list( + bit_width = function() FixedWidthType__bit_width(self) + ) ) Int8 <- R6Class("Int8", inherit = FixedWidthType) @@ -179,146 +176,126 @@ UInt64 <- R6Class("UInt64", inherit = FixedWidthType) Float16 <- R6Class("Float16", inherit = FixedWidthType) Float32 <- R6Class("Float32", inherit = FixedWidthType) Float64 <- R6Class("Float64", - inherit = FixedWidthType, - private = list( - call_name = function() "float64" - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) call2("float64", .ns = if(namespace) "arrow") + ) ) Boolean <- R6Class("Boolean", inherit = FixedWidthType) Utf8 <- R6Class("Utf8", - inherit = DataType, - private = list( - call_name = function() "utf8" - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("utf8", .ns = if(namespace) "arrow") + ) ) LargeUtf8 <- R6Class("LargeUtf8", - inherit = DataType, - private = list( - call_name = function() "large_utf8" - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("large_utf8", .ns = if(namespace) "arrow") + ) ) Binary <- R6Class("Binary", - inherit = DataType, - private = list( - call_name = function() "binary" - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("binary", .ns = if(namespace) "arrow") + ) ) LargeBinary <- R6Class("LargeBinary", - inherit = DataType, - private = list( - call_name = function() "large_binary" - ) + inherit = DataType, public = list( + code = function(namespace = FALSE) call2("large_binary", .ns = if(namespace) "arrow") + ) ) FixedSizeBinary <- R6Class("FixedSizeBinary", - inherit = FixedWidthType, - public = list( - byte_width = function() FixedSizeBinary__byte_width(self), - code = function(explicit_pkg_name = FALSE) { - call2(private$call_name(), byte_width = self$byte_width(), .ns=get_pkg_ns(explicit_pkg_name)) - } - ), - private = list( - call_name = function() "fixed_size_binary" - ) + inherit = FixedWidthType, + public = list( + byte_width = function() FixedSizeBinary__byte_width(self), + code = function(namespace = FALSE) call2("fixed_size_binary", byte_width = self$byte_width(), .ns = if(namespace) "arrow") + ) ) DateType <- R6Class("DateType", - inherit = FixedWidthType, - public = list( - unit = function() DateType__unit(self) - ), - private = list( - call_name = function() tolower(self$name) - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow"), + unit = function() DateType__unit(self) + ) ) Date32 <- R6Class("Date32", inherit = DateType) Date64 <- R6Class("Date64", inherit = DateType) TimeType <- R6Class("TimeType", - inherit = FixedWidthType, - public = list( - unit = function() TimeType__unit(self) - ) + inherit = FixedWidthType, + public = list( + unit = function() TimeType__unit(self) + ) ) Time32 <- R6Class("Time32", - inherit = TimeType, - public = list( - code = function(explicit_pkg_name = FALSE) { - unit <- if (self$unit() == TimeUnit$MILLI) { - "ms" - } else { - "s" - } - call2(private$call_name(), unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) - } - ), - private = list( - call_name = function() "time32" - ) + inherit = TimeType, + public = list( + code = function(namespace = FALSE) { + unit <- if (self$unit() == TimeUnit$MILLI) { + "ms" + } else { + "s" + } + call2("time32", unit = unit, .ns = if(namespace) "arrow") + } + ) ) Time64 <- R6Class("Time64", - inherit = TimeType, - public = list( - code = function(explicit_pkg_name = FALSE) { - unit <- if (self$unit() == TimeUnit$NANO) { - "ns" - } else { - "us" - } - call2(private$call_name(), unit = unit, .ns=get_pkg_ns(explicit_pkg_name)) - } - ), - private = list( - call_name = function() "time64" - ) + inherit = TimeType, + public = list( + code = function(namespace = FALSE) { + unit <- if (self$unit() == TimeUnit$NANO) { + "ns" + } else { + "us" + } + call2("time64", unit = unit, .ns = if(namespace) "arrow") + } + ) ) DurationType <- R6Class("DurationType", - inherit = FixedWidthType, - public = list( - unit = function() DurationType__unit(self) - ) + inherit = FixedWidthType, + public = list( + unit = function() DurationType__unit(self) + ) ) Null <- R6Class("Null", - inherit = DataType, - private = list( - call_name = function() "null" - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("null", .ns = if(namespace) "arrow") + ) ) Timestamp <- R6Class("Timestamp", - inherit = FixedWidthType, - public = list( - code = function(explicit_pkg_name = FALSE) { - unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] - tz <- self$timezone() - if (identical(tz, "")) { - call2(private$call_name(), unit = unit, .ns = get_pkg_ns(explicit_pkg_name)) - } else { - call2(private$call_name(), unit = unit, timezone = tz, .ns = get_pkg_ns(explicit_pkg_name)) - } - }, - timezone = function() TimestampType__timezone(self), - unit = function() TimestampType__unit(self) - ), - private = list( - call_name = function() "timestamp" - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) { + unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] + tz <- self$timezone() + if (identical(tz, "")) { + call2("timestamp", unit = unit, .ns = if(namespace) "arrow") + } else { + call2("timestamp", unit = unit, timezone = tz, .ns = if(namespace) "arrow") + } + }, + timezone = function() TimestampType__timezone(self), + unit = function() TimestampType__unit(self) + ) ) DecimalType <- R6Class("DecimalType", - inherit = FixedWidthType, - public = list( - code = function(explicit_pkg_name = FALSE) { - call2(private$call_name(), precision = self$precision(), scale = self$scale(), .ns = get_pkg_ns(explicit_pkg_name)) - }, - precision = function() DecimalType__precision(self), - scale = function() DecimalType__scale(self) - ), - private = list( - call_name = function() "decimal" - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) { + call2("decimal", precision = self$precision(), scale = self$scale(), .ns = if(namespace) "arrow") + }, + precision = function() DecimalType__precision(self), + scale = function() DecimalType__scale(self) + ) ) Decimal128Type <- R6Class("Decimal128Type", inherit = DecimalType) @@ -645,22 +622,19 @@ check_decimal_args <- function(precision, scale) { } StructType <- R6Class("StructType", - inherit = NestedType, - public = list( - code = function(explicit_pkg_name = FALSE) { - field_names <- StructType__field_names(self) - codes <- map(field_names, function(name) { - self$GetFieldByName(name)$type$code(explicit_pkg_name) - }) - codes <- set_names(codes, field_names) - call2(private$call_name(), !!!codes, .ns = get_pkg_ns(explicit_pkg_name)) - }, - GetFieldByName = function(name) StructType__GetFieldByName(self, name), - GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) - ), - private = list( - call_name = function() "struct" - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + field_names <- StructType__field_names(self) + codes <- map(field_names, function(name) { + self$GetFieldByName(name)$type$code(namespace) + }) + codes <- set_names(codes, field_names) + call2("struct", !!!codes, .ns = if(namespace) "arrow") + }, + GetFieldByName = function(name) StructType__GetFieldByName(self, name), + GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) + ) ) StructType$create <- function(...) struct__(.fields(list(...))) @@ -672,19 +646,16 @@ struct <- StructType$create names.StructType <- function(x) StructType__field_names(x) ListType <- R6Class("ListType", - inherit = NestedType, - public = list( - code = function(explicit_pkg_name = FALSE) { - call2(private$call_name(), self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) - } - ), - active = list( - value_field = function() ListType__value_field(self), - value_type = function() ListType__value_type(self) - ), - private = list( - call_name = function() "list_of" - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + call2("list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") + } + ), + active = list( + value_field = function() ListType__value_field(self), + value_type = function() ListType__value_type(self) + ) ) #' @rdname data-type @@ -692,19 +663,16 @@ ListType <- R6Class("ListType", list_of <- function(type) list__(type) LargeListType <- R6Class("LargeListType", - inherit = NestedType, - public = list( - code = function(explicit_pkg_name = FALSE) { - call2(private$call_name(), self$value_type$code(explicit_pkg_name), .ns = get_pkg_ns(explicit_pkg_name)) - } - ), - active = list( - value_field = function() LargeListType__value_field(self), - value_type = function() LargeListType__value_type(self) - ), - private = list( - call_name = function() "large_list_of" - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + call2("large_list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") + } + ), + active = list( + value_field = function() LargeListType__value_field(self), + value_type = function() LargeListType__value_type(self) + ) ) #' @rdname data-type @@ -714,20 +682,17 @@ large_list_of <- function(type) large_list__(type) #' @rdname data-type #' @export FixedSizeListType <- R6Class("FixedSizeListType", - inherit = NestedType, - public = list( - code = function(explicit_pkg_name = FALSE) { - call2(private$call_name(), self$value_type$code(explicit_pkg_name), list_size = self$list_size, .ns = get_pkg_ns(explicit_pkg_name)) - } - ), - active = list( - value_field = function() FixedSizeListType__value_field(self), - value_type = function() FixedSizeListType__value_type(self), - list_size = function() FixedSizeListType__list_size(self) - ), - private = list( - call_name = function() "fixed_size_list_of" - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + call2("fixed_size_list_of", self$value_type$code(namespace), list_size = self$list_size, .ns = if(namespace) "arrow") + } + ), + active = list( + value_field = function() FixedSizeListType__value_field(self), + value_type = function() FixedSizeListType__value_type(self), + list_size = function() FixedSizeListType__list_size(self) + ) ) #' @rdname data-type @@ -737,14 +702,14 @@ fixed_size_list_of <- function(type, list_size) fixed_size_list__(type, list_siz #' @rdname data-type #' @export MapType <- R6Class("MapType", - inherit = ListType, - active = list( - key_field = function() MapType__key_field(self), - item_field = function() MapType__item_field(self), - key_type = function() MapType__key_type(self), - item_type = function() MapType__item_type(self), - keys_sorted = function() MapType__keys_sorted(self) - ) + inherit = ListType, + active = list( + key_field = function() MapType__key_field(self), + item_field = function() MapType__item_field(self), + key_type = function() MapType__key_type(self), + item_type = function() MapType__item_type(self), + keys_sorted = function() MapType__keys_sorted(self) + ) ) #' @rdname data-type @@ -770,47 +735,47 @@ canonical_type_str <- function(type_str) { stop("Cannot interpret string representations of data types that have parameters", call. = FALSE) } switch(type_str, - int8 = "int8", - int16 = "int16", - int32 = "int32", - int64 = "int64", - uint8 = "uint8", - uint16 = "uint16", - uint32 = "uint32", - uint64 = "uint64", - float16 = "halffloat", - halffloat = "halffloat", - float32 = "float", - float = "float", - float64 = "double", - double = "double", - boolean = "bool", - bool = "bool", - utf8 = "string", - large_utf8 = "large_string", - large_string = "large_string", - binary = "binary", - large_binary = "large_binary", - fixed_size_binary = "fixed_size_binary", - string = "string", - date32 = "date32", - date64 = "date64", - time32 = "time32", - time64 = "time64", - null = "null", - timestamp = "timestamp", - decimal128 = "decimal128", - decimal256 = "decimal256", - struct = "struct", - list_of = "list", - list = "list", - large_list_of = "large_list", - large_list = "large_list", - fixed_size_list_of = "fixed_size_list", - fixed_size_list = "fixed_size_list", - map_of = "map", - duration = "duration", - stop("Unrecognized string representation of data type", call. = FALSE) + int8 = "int8", + int16 = "int16", + int32 = "int32", + int64 = "int64", + uint8 = "uint8", + uint16 = "uint16", + uint32 = "uint32", + uint64 = "uint64", + float16 = "halffloat", + halffloat = "halffloat", + float32 = "float", + float = "float", + float64 = "double", + double = "double", + boolean = "bool", + bool = "bool", + utf8 = "string", + large_utf8 = "large_string", + large_string = "large_string", + binary = "binary", + large_binary = "large_binary", + fixed_size_binary = "fixed_size_binary", + string = "string", + date32 = "date32", + date64 = "date64", + time32 = "time32", + time64 = "time64", + null = "null", + timestamp = "timestamp", + decimal128 = "decimal128", + decimal256 = "decimal256", + struct = "struct", + list_of = "list", + list = "list", + large_list_of = "large_list", + large_list = "large_list", + fixed_size_list_of = "fixed_size_list", + fixed_size_list = "fixed_size_list", + map_of = "map", + duration = "duration", + stop("Unrecognized string representation of data type", call. = FALSE) ) } diff --git a/r/R/util.R b/r/R/util.R index 7b49537164ffa..a7cb5b3792d29 100644 --- a/r/R/util.R +++ b/r/R/util.R @@ -262,10 +262,3 @@ check_named_cols <- function(df) { ) } } - -#' Prepares the value of `.ns` to use with `rlang::call2()` -#' -#' @param explicit_pkg_name Whether to include the package name or not. -get_pkg_ns <- function(explicit_pkg_name) { - if(explicit_pkg_name) getPackageName() else NULL -} diff --git a/r/man/DataType-class.Rd b/r/man/DataType-class.Rd index 1c00422b3e4f0..69e19153038c6 100644 --- a/r/man/DataType-class.Rd +++ b/r/man/DataType-class.Rd @@ -13,7 +13,7 @@ DataType class \item \verb{$ToString()}: String representation of the DataType \item \verb{$Equals(other)}: Is the DataType equal to \code{other} \item \verb{$fields()}: The children fields associated with this type -\item \verb{$code(explicit_pkg_name)}: Produces an R call of the data type. Use \code{explicit_pkg_name=TRUE} to call with \verb{arrow::}. +\item \verb{$code(namespace)}: Produces an R call for the schema. Use \code{namespace=TRUE} to call with \verb{arrow::}. } There are also some active bindings: diff --git a/r/man/Schema-class.Rd b/r/man/Schema-class.Rd index a85e2e11edce1..a0c790ca7b705 100644 --- a/r/man/Schema-class.Rd +++ b/r/man/Schema-class.Rd @@ -24,7 +24,7 @@ Many Arrow objects, including \link{Table} and \link{Dataset}, have a \verb{$sch \item \verb{$WithMetadata(metadata)}: returns a new \code{Schema} with the key-value \code{metadata} set. Note that all list elements in \code{metadata} will be coerced to \code{character}. -\item \verb{$code(explicit_pkg_name)}: Produces an R call for the schema. Use \code{explicit_pkg_name=TRUE} to call with \verb{arrow::}. +\item \verb{$code(namespace)}: Produces an R call for the schema. Use \code{namespace=TRUE} to call with \verb{arrow::}. } } diff --git a/r/man/get_pkg_ns.Rd b/r/man/get_pkg_ns.Rd deleted file mode 100644 index b018e6f15623a..0000000000000 --- a/r/man/get_pkg_ns.Rd +++ /dev/null @@ -1,14 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/util.R -\name{get_pkg_ns} -\alias{get_pkg_ns} -\title{Prepares the value of \code{.ns} to use with \code{rlang::call2()}} -\usage{ -get_pkg_ns(explicit_pkg_name) -} -\arguments{ -\item{explicit_pkg_name}{Whether to include the package name or not.} -} -\description{ -Prepares the value of \code{.ns} to use with \code{rlang::call2()} -} diff --git a/r/tests/testthat/helper-roundtrip.R b/r/tests/testthat/helper-roundtrip.R index 57132542efb6d..54e3e21fc4450 100644 --- a/r/tests/testthat/helper-roundtrip.R +++ b/r/tests/testthat/helper-roundtrip.R @@ -43,6 +43,6 @@ expect_array_roundtrip <- function(x, type, as = NULL) { invisible(a) } -expect_code_roundtrip <- function(x, explicit_pkg_name = FALSE, ...) { - expect_equal(eval(x$code(explicit_pkg_name)), x, ...) +expect_code_roundtrip <- function(x, namespace = FALSE, ...) { + expect_equal(eval(x$code(namespace)), x, ...) } diff --git a/r/tests/testthat/test-schema.R b/r/tests/testthat/test-schema.R index 5df0573f6443f..26bbae931b07e 100644 --- a/r/tests/testthat/test-schema.R +++ b/r/tests/testthat/test-schema.R @@ -41,7 +41,8 @@ test_that("Schema print method", { test_that("Schema$code()", { schema_obj <- schema(a = int32(), b = struct(c = double(), d = utf8()), e = list_of(binary())) expect_code_roundtrip(schema_obj) - expect_code_roundtrip(schema_obj, explicit_pkg_name = TRUE) + + expect_no_match(as.character(schema_obj$code()), "arrow::", fixed=TRUE) skip_if(packageVersion("rlang") < "1") expect_error( @@ -50,6 +51,13 @@ test_that("Schema$code()", { ) }) +test_that("Schema$code(namespace=TRUE)", { + schema_obj <- schema(a = int32(), b = struct(c = double(), d = utf8()), e = list_of(binary())) + expect_code_roundtrip(schema_obj, namespace = TRUE) + + expect_match(as.character(schema_obj$code(TRUE)), "^arrow[:][:]") +}) + test_that("Schema with non-nullable fields", { expect_output( print( diff --git a/r/tests/testthat/test-type.R b/r/tests/testthat/test-type.R index f1183c7eba751..e6aa195a8f347 100644 --- a/r/tests/testthat/test-type.R +++ b/r/tests/testthat/test-type.R @@ -345,7 +345,7 @@ test_that("infer_type() infers type for lists starting with NULL - ARROW-17639", }) -test_that("code() works for data types without arguments",{ +test_that("code(namespace) works for data types without arguments",{ # Names encode type aliases. # No names means the type alias matches the string type_strs <- c( @@ -365,7 +365,7 @@ test_that("code() works for data types without arguments",{ type_obj <- eval(call2(type_str, .ns=getPackageName())) expect_code_roundtrip(type_obj, info = type_str) - expect_code_roundtrip(type_obj, explicit_pkg_name = TRUE, info = type_str) + expect_code_roundtrip(type_obj, namespace = TRUE, info = type_str) type_code <- as.character(type_obj$code())[1] # Ignore units in time types. type_code_with_ns <- as.character(type_obj$code(TRUE))[1] # Ignore units in time types. @@ -377,7 +377,7 @@ test_that("code() works for data types without arguments",{ }) -test_that("code() works for simple data types with arguments",{ +test_that("code(namespace) works for simple data types with arguments",{ types_with_args <- list( # type_str, args, type_alias list("fixed_size_binary", list(42), "fixed_size_binary"), @@ -394,15 +394,15 @@ test_that("code() works for simple data types with arguments",{ if(type_str == type_alias) { expect_code_roundtrip(type_obj, info = type_str) - expect_code_roundtrip(type_obj, explicit_pkg_name = TRUE, info = type_str) + expect_code_roundtrip(type_obj, namespace = TRUE, info = type_str) } type_code <- as.character(type_obj$code()) type_code_with_ns <- as.character(type_obj$code(TRUE)) # test info - build_test_info <- function(test_str, explicit_pkg_name=FALSE) { - glue::glue("`{type_str}` {test_str} (explicit_pkg_name={explicit_pkg_name})") + build_test_info <- function(test_str, namespace=FALSE) { + glue::glue("`{type_str}` {test_str} (namespace={namespace})") } # type name @@ -416,7 +416,7 @@ test_that("code() works for simple data types with arguments",{ purrr::walk(types_with_args, evaluate_type_with_arg) }) -test_that("code() works for nested_types",{ +test_that("code(namespace) works for nested_types",{ # Nested Types nested_types <- list( list("struct", list(foo=int32())), @@ -431,15 +431,15 @@ test_that("code() works for nested_types",{ type_obj <- eval(call2(type_str, !!!args, .ns=getPackageName())) expect_code_roundtrip(type_obj, info = type_str) - expect_code_roundtrip(type_obj, explicit_pkg_name = TRUE, info = type_str) + expect_code_roundtrip(type_obj, namespace = TRUE, info = type_str) type_code <- as.character(type_obj$code()) type_code_with_ns <- as.character(type_obj$code(TRUE)) # test info - build_test_info <- function(test_str, explicit_pkg_name=FALSE) { - glue::glue("`{type_str}` {test_str} (explicit_pkg_name={explicit_pkg_name})") + build_test_info <- function(test_str, namespace=FALSE) { + glue::glue("`{type_str}` {test_str} (namespace={namespace})") } # type name @@ -448,9 +448,9 @@ test_that("code() works for nested_types",{ info=build_test_info("type name", TRUE)) # first arg (also a type) - build_expected_str <- function(explicit_pkg_name=FALSE) { + build_expected_str <- function(namespace=FALSE) { paste0( - as.character(args[[1]]$code(explicit_pkg_name)), + as.character(args[[1]]$code(namespace)), "()" # The () is kept in arguments. ) } @@ -468,7 +468,7 @@ test_that("code() works for nested_types",{ }) -test_that("code() works for map_of",{ +test_that("code(namespace) works for map_of",{ type_str <- "map_of" args <- list(string(), string()) @@ -478,8 +478,8 @@ test_that("code() works for map_of",{ # test info - build_test_info <- function(test_str, explicit_pkg_name=FALSE) { - glue::glue("`{type_str}` {test_str} (explicit_pkg_name={explicit_pkg_name})") + build_test_info <- function(test_str, namespace=FALSE) { + glue::glue("`{type_str}` {test_str} (namespace={namespace})") } # list_of @@ -488,8 +488,8 @@ test_that("code() works for map_of",{ info=build_test_info("list_of", TRUE)) # struct argument - expect_struct_code_matches <- function(struct_code, explicit_pkg_name=FALSE) { - get_code_str <- function(obj) as.character(obj$code(explicit_pkg_name)) + expect_struct_code_matches <- function(struct_code, namespace=FALSE) { + get_code_str <- function(obj) as.character(obj$code(namespace)) expect_true(grepl(get_code_str(arrow::struct()), struct_code, fixed = TRUE)) for(arg in args) { From 629e5a44023349ca6af3ca1a8315ca3285617143 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Thu, 12 Oct 2023 22:23:03 -0700 Subject: [PATCH 10/17] Revert indentation changes --- r/R/type.R | 408 ++++++++++++++++++++++++++--------------------------- 1 file changed, 204 insertions(+), 204 deletions(-) diff --git a/r/R/type.R b/r/R/type.R index 0bfc8c20d1ea1..4924d6bc8f706 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -39,25 +39,25 @@ #' @name DataType #' @seealso [`data-type`] DataType <- R6Class("DataType", - inherit = ArrowObject, - public = list( - ToString = function() { - DataType__ToString(self) - }, - Equals = function(other, check_metadata = FALSE, ...) { - inherits(other, "DataType") && DataType__Equals(self, other, isTRUE(check_metadata)) - }, - fields = function() { - DataType__fields(self) - }, - export_to_c = function(ptr) ExportType(self, ptr), - code = function(namespace = FALSE) call2("stop", paste0("Unsupported type: <", self$ToString(), ">.")) - ), - active = list( - id = function() DataType__id(self), - name = function() DataType__name(self), - num_fields = function() DataType__num_fields(self) - ) + inherit = ArrowObject, + public = list( + ToString = function() { + DataType__ToString(self) + }, + Equals = function(other, check_metadata = FALSE, ...) { + inherits(other, "DataType") && DataType__Equals(self, other, isTRUE(check_metadata)) + }, + fields = function() { + DataType__fields(self) + }, + export_to_c = function(ptr) ExportType(self, ptr), + code = function(namespace = FALSE) call("stop", paste0("Unsupported type: <", self$ToString(), ">.")) + ), + active = list( + id = function() DataType__id(self), + name = function() DataType__name(self), + num_fields = function() DataType__num_fields(self) + ) ) #' @include arrowExports.R @@ -156,13 +156,13 @@ infer_type.Expression <- function(x, ...) x$type() #' @rdname FixedWidthType #' @name FixedWidthType FixedWidthType <- R6Class("FixedWidthType", - inherit = DataType, - public = list( - code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow") - ), - active = list( - bit_width = function() FixedWidthType__bit_width(self) - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow") + ), + active = list( + bit_width = function() FixedWidthType__bit_width(self) + ) ) Int8 <- R6Class("Int8", inherit = FixedWidthType) @@ -176,126 +176,126 @@ UInt64 <- R6Class("UInt64", inherit = FixedWidthType) Float16 <- R6Class("Float16", inherit = FixedWidthType) Float32 <- R6Class("Float32", inherit = FixedWidthType) Float64 <- R6Class("Float64", - inherit = FixedWidthType, - public = list( - code = function(namespace = FALSE) call2("float64", .ns = if(namespace) "arrow") - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) call2("float64", .ns = if(namespace) "arrow") + ) ) Boolean <- R6Class("Boolean", inherit = FixedWidthType) Utf8 <- R6Class("Utf8", - inherit = DataType, - public = list( - code = function(namespace = FALSE) call2("utf8", .ns = if(namespace) "arrow") - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("utf8", .ns = if(namespace) "arrow") + ) ) LargeUtf8 <- R6Class("LargeUtf8", - inherit = DataType, - public = list( - code = function(namespace = FALSE) call2("large_utf8", .ns = if(namespace) "arrow") - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("large_utf8", .ns = if(namespace) "arrow") + ) ) Binary <- R6Class("Binary", - inherit = DataType, - public = list( - code = function(namespace = FALSE) call2("binary", .ns = if(namespace) "arrow") - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("binary", .ns = if(namespace) "arrow") + ) ) LargeBinary <- R6Class("LargeBinary", - inherit = DataType, public = list( - code = function(namespace = FALSE) call2("large_binary", .ns = if(namespace) "arrow") - ) + inherit = DataType, public = list( + code = function(namespace = FALSE) call2("large_binary", .ns = if(namespace) "arrow") + ) ) FixedSizeBinary <- R6Class("FixedSizeBinary", - inherit = FixedWidthType, - public = list( - byte_width = function() FixedSizeBinary__byte_width(self), - code = function(namespace = FALSE) call2("fixed_size_binary", byte_width = self$byte_width(), .ns = if(namespace) "arrow") - ) + inherit = FixedWidthType, + public = list( + byte_width = function() FixedSizeBinary__byte_width(self), + code = function(namespace = FALSE) call2("fixed_size_binary", byte_width = self$byte_width(), .ns = if(namespace) "arrow") + ) ) DateType <- R6Class("DateType", - inherit = FixedWidthType, - public = list( - code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow"), - unit = function() DateType__unit(self) - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow"), + unit = function() DateType__unit(self) + ) ) Date32 <- R6Class("Date32", inherit = DateType) Date64 <- R6Class("Date64", inherit = DateType) TimeType <- R6Class("TimeType", - inherit = FixedWidthType, - public = list( - unit = function() TimeType__unit(self) - ) + inherit = FixedWidthType, + public = list( + unit = function() TimeType__unit(self) + ) ) Time32 <- R6Class("Time32", - inherit = TimeType, - public = list( - code = function(namespace = FALSE) { - unit <- if (self$unit() == TimeUnit$MILLI) { - "ms" - } else { - "s" - } - call2("time32", unit = unit, .ns = if(namespace) "arrow") - } - ) + inherit = TimeType, + public = list( + code = function(namespace = FALSE) { + unit <- if (self$unit() == TimeUnit$MILLI) { + "ms" + } else { + "s" + } + call2("time32", unit = unit, .ns = if(namespace) "arrow") + } + ) ) Time64 <- R6Class("Time64", - inherit = TimeType, - public = list( - code = function(namespace = FALSE) { - unit <- if (self$unit() == TimeUnit$NANO) { - "ns" - } else { - "us" - } - call2("time64", unit = unit, .ns = if(namespace) "arrow") - } - ) + inherit = TimeType, + public = list( + code = function(namespace = FALSE) { + unit <- if (self$unit() == TimeUnit$NANO) { + "ns" + } else { + "us" + } + call2("time64", unit = unit, .ns = if(namespace) "arrow") + } + ) ) DurationType <- R6Class("DurationType", - inherit = FixedWidthType, - public = list( - unit = function() DurationType__unit(self) - ) + inherit = FixedWidthType, + public = list( + unit = function() DurationType__unit(self) + ) ) Null <- R6Class("Null", - inherit = DataType, - public = list( - code = function(namespace = FALSE) call2("null", .ns = if(namespace) "arrow") - ) + inherit = DataType, + public = list( + code = function(namespace = FALSE) call2("null", .ns = if(namespace) "arrow") + ) ) Timestamp <- R6Class("Timestamp", - inherit = FixedWidthType, - public = list( - code = function(namespace = FALSE) { - unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] - tz <- self$timezone() - if (identical(tz, "")) { - call2("timestamp", unit = unit, .ns = if(namespace) "arrow") - } else { - call2("timestamp", unit = unit, timezone = tz, .ns = if(namespace) "arrow") - } - }, - timezone = function() TimestampType__timezone(self), - unit = function() TimestampType__unit(self) - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) { + unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] + tz <- self$timezone() + if (identical(tz, "")) { + call2("timestamp", unit = unit, .ns = if(namespace) "arrow") + } else { + call2("timestamp", unit = unit, timezone = tz, .ns = if(namespace) "arrow") + } + }, + timezone = function() TimestampType__timezone(self), + unit = function() TimestampType__unit(self) + ) ) DecimalType <- R6Class("DecimalType", - inherit = FixedWidthType, - public = list( - code = function(namespace = FALSE) { - call2("decimal", precision = self$precision(), scale = self$scale(), .ns = if(namespace) "arrow") - }, - precision = function() DecimalType__precision(self), - scale = function() DecimalType__scale(self) - ) + inherit = FixedWidthType, + public = list( + code = function(namespace = FALSE) { + call2("decimal", precision = self$precision(), scale = self$scale(), .ns = if(namespace) "arrow") + }, + precision = function() DecimalType__precision(self), + scale = function() DecimalType__scale(self) + ) ) Decimal128Type <- R6Class("Decimal128Type", inherit = DecimalType) @@ -622,19 +622,19 @@ check_decimal_args <- function(precision, scale) { } StructType <- R6Class("StructType", - inherit = NestedType, - public = list( - code = function(namespace = FALSE) { - field_names <- StructType__field_names(self) - codes <- map(field_names, function(name) { - self$GetFieldByName(name)$type$code(namespace) - }) - codes <- set_names(codes, field_names) - call2("struct", !!!codes, .ns = if(namespace) "arrow") - }, - GetFieldByName = function(name) StructType__GetFieldByName(self, name), - GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + field_names <- StructType__field_names(self) + codes <- map(field_names, function(name) { + self$GetFieldByName(name)$type$code(namespace) + }) + codes <- set_names(codes, field_names) + call2("struct", !!!codes, .ns = if(namespace) "arrow") + }, + GetFieldByName = function(name) StructType__GetFieldByName(self, name), + GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) + ) ) StructType$create <- function(...) struct__(.fields(list(...))) @@ -646,16 +646,16 @@ struct <- StructType$create names.StructType <- function(x) StructType__field_names(x) ListType <- R6Class("ListType", - inherit = NestedType, - public = list( - code = function(namespace = FALSE) { - call2("list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") - } - ), - active = list( - value_field = function() ListType__value_field(self), - value_type = function() ListType__value_type(self) - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + call2("list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") + } + ), + active = list( + value_field = function() ListType__value_field(self), + value_type = function() ListType__value_type(self) + ) ) #' @rdname data-type @@ -663,16 +663,16 @@ ListType <- R6Class("ListType", list_of <- function(type) list__(type) LargeListType <- R6Class("LargeListType", - inherit = NestedType, - public = list( - code = function(namespace = FALSE) { - call2("large_list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") - } - ), - active = list( - value_field = function() LargeListType__value_field(self), - value_type = function() LargeListType__value_type(self) - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + call2("large_list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") + } + ), + active = list( + value_field = function() LargeListType__value_field(self), + value_type = function() LargeListType__value_type(self) + ) ) #' @rdname data-type @@ -682,17 +682,17 @@ large_list_of <- function(type) large_list__(type) #' @rdname data-type #' @export FixedSizeListType <- R6Class("FixedSizeListType", - inherit = NestedType, - public = list( - code = function(namespace = FALSE) { - call2("fixed_size_list_of", self$value_type$code(namespace), list_size = self$list_size, .ns = if(namespace) "arrow") - } - ), - active = list( - value_field = function() FixedSizeListType__value_field(self), - value_type = function() FixedSizeListType__value_type(self), - list_size = function() FixedSizeListType__list_size(self) - ) + inherit = NestedType, + public = list( + code = function(namespace = FALSE) { + call2("fixed_size_list_of", self$value_type$code(namespace), list_size = self$list_size, .ns = if(namespace) "arrow") + } + ), + active = list( + value_field = function() FixedSizeListType__value_field(self), + value_type = function() FixedSizeListType__value_type(self), + list_size = function() FixedSizeListType__list_size(self) + ) ) #' @rdname data-type @@ -702,14 +702,14 @@ fixed_size_list_of <- function(type, list_size) fixed_size_list__(type, list_siz #' @rdname data-type #' @export MapType <- R6Class("MapType", - inherit = ListType, - active = list( - key_field = function() MapType__key_field(self), - item_field = function() MapType__item_field(self), - key_type = function() MapType__key_type(self), - item_type = function() MapType__item_type(self), - keys_sorted = function() MapType__keys_sorted(self) - ) + inherit = ListType, + active = list( + key_field = function() MapType__key_field(self), + item_field = function() MapType__item_field(self), + key_type = function() MapType__key_type(self), + item_type = function() MapType__item_type(self), + keys_sorted = function() MapType__keys_sorted(self) + ) ) #' @rdname data-type @@ -735,47 +735,47 @@ canonical_type_str <- function(type_str) { stop("Cannot interpret string representations of data types that have parameters", call. = FALSE) } switch(type_str, - int8 = "int8", - int16 = "int16", - int32 = "int32", - int64 = "int64", - uint8 = "uint8", - uint16 = "uint16", - uint32 = "uint32", - uint64 = "uint64", - float16 = "halffloat", - halffloat = "halffloat", - float32 = "float", - float = "float", - float64 = "double", - double = "double", - boolean = "bool", - bool = "bool", - utf8 = "string", - large_utf8 = "large_string", - large_string = "large_string", - binary = "binary", - large_binary = "large_binary", - fixed_size_binary = "fixed_size_binary", - string = "string", - date32 = "date32", - date64 = "date64", - time32 = "time32", - time64 = "time64", - null = "null", - timestamp = "timestamp", - decimal128 = "decimal128", - decimal256 = "decimal256", - struct = "struct", - list_of = "list", - list = "list", - large_list_of = "large_list", - large_list = "large_list", - fixed_size_list_of = "fixed_size_list", - fixed_size_list = "fixed_size_list", - map_of = "map", - duration = "duration", - stop("Unrecognized string representation of data type", call. = FALSE) + int8 = "int8", + int16 = "int16", + int32 = "int32", + int64 = "int64", + uint8 = "uint8", + uint16 = "uint16", + uint32 = "uint32", + uint64 = "uint64", + float16 = "halffloat", + halffloat = "halffloat", + float32 = "float", + float = "float", + float64 = "double", + double = "double", + boolean = "bool", + bool = "bool", + utf8 = "string", + large_utf8 = "large_string", + large_string = "large_string", + binary = "binary", + large_binary = "large_binary", + fixed_size_binary = "fixed_size_binary", + string = "string", + date32 = "date32", + date64 = "date64", + time32 = "time32", + time64 = "time64", + null = "null", + timestamp = "timestamp", + decimal128 = "decimal128", + decimal256 = "decimal256", + struct = "struct", + list_of = "list", + list = "list", + large_list_of = "large_list", + large_list = "large_list", + fixed_size_list_of = "fixed_size_list", + fixed_size_list = "fixed_size_list", + map_of = "map", + duration = "duration", + stop("Unrecognized string representation of data type", call. = FALSE) ) } From c4b1464e7bf079300ab19b86aea81e7af7eb7acd Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Thu, 12 Oct 2023 22:25:15 -0700 Subject: [PATCH 11/17] Fix incorrect documentation --- r/R/type.R | 2 +- r/man/DataType-class.Rd | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/r/R/type.R b/r/R/type.R index 4924d6bc8f706..f1f472b858e81 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -27,7 +27,7 @@ #' - `$ToString()`: String representation of the DataType #' - `$Equals(other)`: Is the DataType equal to `other` #' - `$fields()`: The children fields associated with this type -#' - `$code(namespace)`: Produces an R call for the schema. Use `namespace=TRUE` to call with `arrow::`. +#' - `$code(namespace)`: Produces an R call of the data type. Use `namespace=TRUE` to call with `arrow::`. #' #' There are also some active bindings: #' - `$id`: integer Arrow type id. diff --git a/r/man/DataType-class.Rd b/r/man/DataType-class.Rd index 69e19153038c6..5c17c072f92b5 100644 --- a/r/man/DataType-class.Rd +++ b/r/man/DataType-class.Rd @@ -13,7 +13,7 @@ DataType class \item \verb{$ToString()}: String representation of the DataType \item \verb{$Equals(other)}: Is the DataType equal to \code{other} \item \verb{$fields()}: The children fields associated with this type -\item \verb{$code(namespace)}: Produces an R call for the schema. Use \code{namespace=TRUE} to call with \verb{arrow::}. +\item \verb{$code(namespace)}: Produces an R call of the data type. Use \code{namespace=TRUE} to call with \verb{arrow::}. } There are also some active bindings: From c0a80a9c7c28d383b22a4ddbc6cefd6eb9fce511 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Thu, 12 Oct 2023 22:42:18 -0700 Subject: [PATCH 12/17] Add namespace to DictionaryType$code(namespace) --- r/R/dictionary.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/r/R/dictionary.R b/r/R/dictionary.R index df94ada035e14..09cdd3897f8fe 100644 --- a/r/R/dictionary.R +++ b/r/R/dictionary.R @@ -35,18 +35,18 @@ DictionaryType <- R6Class("DictionaryType", ToString = function() { prettier_dictionary_type(DataType__ToString(self)) }, - code = function() { + code = function(namespace = FALSE) { details <- list() if (self$index_type != int32()) { - details$index_type <- self$index_type$code() + details$index_type <- self$index_type$code(namespace) } if (self$value_type != utf8()) { - details$value_type <- self$value_type$code() + details$value_type <- self$value_type$code(namespace) } if (isTRUE(self$ordered)) { details$ordered <- TRUE } - call2("dictionary", !!!details) + call2("dictionary", !!!details, .ns = if(namespace) "arrow") } ), active = list( From 74a5a402d1060e208a1e9fd2a45b263c9ccc3a7b Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Sat, 14 Oct 2023 16:15:18 -0700 Subject: [PATCH 13/17] Update style --- r/R/schema.R | 2 +- r/R/type.R | 36 ++++++++++++++++++------------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/r/R/schema.R b/r/R/schema.R index f95af9fd2c195..68a251dc24db8 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -114,7 +114,7 @@ Schema <- R6Class("Schema", field$type$code(namespace) }) codes <- set_names(codes, names) - call2("schema", !!!codes, .ns = if(namespace) "arrow") + call2("schema", !!!codes, .ns = if (namespace) "arrow") }, WithNames = function(names) { if (!inherits(names, "character")) { diff --git a/r/R/type.R b/r/R/type.R index f1f472b858e81..b7ce05814dc14 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -158,7 +158,7 @@ infer_type.Expression <- function(x, ...) x$type() FixedWidthType <- R6Class("FixedWidthType", inherit = DataType, public = list( - code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2(tolower(self$name), .ns = if (namespace) "arrow") ), active = list( bit_width = function() FixedWidthType__bit_width(self) @@ -178,45 +178,45 @@ Float32 <- R6Class("Float32", inherit = FixedWidthType) Float64 <- R6Class("Float64", inherit = FixedWidthType, public = list( - code = function(namespace = FALSE) call2("float64", .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2("float64", .ns = if (namespace) "arrow") ) ) Boolean <- R6Class("Boolean", inherit = FixedWidthType) Utf8 <- R6Class("Utf8", inherit = DataType, public = list( - code = function(namespace = FALSE) call2("utf8", .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2("utf8", .ns = if (namespace) "arrow") ) ) LargeUtf8 <- R6Class("LargeUtf8", inherit = DataType, public = list( - code = function(namespace = FALSE) call2("large_utf8", .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2("large_utf8", .ns = if (namespace) "arrow") ) ) Binary <- R6Class("Binary", inherit = DataType, public = list( - code = function(namespace = FALSE) call2("binary", .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2("binary", .ns = if (namespace) "arrow") ) ) LargeBinary <- R6Class("LargeBinary", inherit = DataType, public = list( - code = function(namespace = FALSE) call2("large_binary", .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2("large_binary", .ns = if (namespace) "arrow") ) ) FixedSizeBinary <- R6Class("FixedSizeBinary", inherit = FixedWidthType, public = list( byte_width = function() FixedSizeBinary__byte_width(self), - code = function(namespace = FALSE) call2("fixed_size_binary", byte_width = self$byte_width(), .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2("fixed_size_binary", byte_width = self$byte_width(), .ns = if (namespace) "arrow") ) ) DateType <- R6Class("DateType", inherit = FixedWidthType, public = list( - code = function(namespace = FALSE) call2(tolower(self$name), .ns = if(namespace) "arrow"), + code = function(namespace = FALSE) call2(tolower(self$name), .ns = if (namespace) "arrow"), unit = function() DateType__unit(self) ) ) @@ -238,7 +238,7 @@ Time32 <- R6Class("Time32", } else { "s" } - call2("time32", unit = unit, .ns = if(namespace) "arrow") + call2("time32", unit = unit, .ns = if (namespace) "arrow") } ) ) @@ -251,7 +251,7 @@ Time64 <- R6Class("Time64", } else { "us" } - call2("time64", unit = unit, .ns = if(namespace) "arrow") + call2("time64", unit = unit, .ns = if (namespace) "arrow") } ) ) @@ -266,7 +266,7 @@ DurationType <- R6Class("DurationType", Null <- R6Class("Null", inherit = DataType, public = list( - code = function(namespace = FALSE) call2("null", .ns = if(namespace) "arrow") + code = function(namespace = FALSE) call2("null", .ns = if (namespace) "arrow") ) ) @@ -277,9 +277,9 @@ Timestamp <- R6Class("Timestamp", unit <- c("s", "ms", "us", "ns")[self$unit() + 1L] tz <- self$timezone() if (identical(tz, "")) { - call2("timestamp", unit = unit, .ns = if(namespace) "arrow") + call2("timestamp", unit = unit, .ns = if (namespace) "arrow") } else { - call2("timestamp", unit = unit, timezone = tz, .ns = if(namespace) "arrow") + call2("timestamp", unit = unit, timezone = tz, .ns = if (namespace) "arrow") } }, timezone = function() TimestampType__timezone(self), @@ -291,7 +291,7 @@ DecimalType <- R6Class("DecimalType", inherit = FixedWidthType, public = list( code = function(namespace = FALSE) { - call2("decimal", precision = self$precision(), scale = self$scale(), .ns = if(namespace) "arrow") + call2("decimal", precision = self$precision(), scale = self$scale(), .ns = if (namespace) "arrow") }, precision = function() DecimalType__precision(self), scale = function() DecimalType__scale(self) @@ -630,7 +630,7 @@ StructType <- R6Class("StructType", self$GetFieldByName(name)$type$code(namespace) }) codes <- set_names(codes, field_names) - call2("struct", !!!codes, .ns = if(namespace) "arrow") + call2("struct", !!!codes, .ns = if (namespace) "arrow") }, GetFieldByName = function(name) StructType__GetFieldByName(self, name), GetFieldIndex = function(name) StructType__GetFieldIndex(self, name) @@ -649,7 +649,7 @@ ListType <- R6Class("ListType", inherit = NestedType, public = list( code = function(namespace = FALSE) { - call2("list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") + call2("list_of", self$value_type$code(namespace), .ns = if (namespace) "arrow") } ), active = list( @@ -666,7 +666,7 @@ LargeListType <- R6Class("LargeListType", inherit = NestedType, public = list( code = function(namespace = FALSE) { - call2("large_list_of", self$value_type$code(namespace), .ns = if(namespace) "arrow") + call2("large_list_of", self$value_type$code(namespace), .ns = if (namespace) "arrow") } ), active = list( @@ -685,7 +685,7 @@ FixedSizeListType <- R6Class("FixedSizeListType", inherit = NestedType, public = list( code = function(namespace = FALSE) { - call2("fixed_size_list_of", self$value_type$code(namespace), list_size = self$list_size, .ns = if(namespace) "arrow") + call2("fixed_size_list_of", self$value_type$code(namespace), list_size = self$list_size, .ns = if (namespace) "arrow") } ), active = list( From ec46e9d3387ce906975c1d480c1f83e51646b7f8 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Sat, 14 Oct 2023 16:20:00 -0700 Subject: [PATCH 14/17] Simplify testing significantly by reverting all testing changes and adding $code(namespace=TRUE) test into `expect_code_roundtrip()`. --- r/tests/testthat/helper-roundtrip.R | 12 ++- r/tests/testthat/test-schema.R | 14 +-- r/tests/testthat/test-type.R | 157 ---------------------------- 3 files changed, 13 insertions(+), 170 deletions(-) diff --git a/r/tests/testthat/helper-roundtrip.R b/r/tests/testthat/helper-roundtrip.R index 54e3e21fc4450..4e22737d2b6fc 100644 --- a/r/tests/testthat/helper-roundtrip.R +++ b/r/tests/testthat/helper-roundtrip.R @@ -43,6 +43,14 @@ expect_array_roundtrip <- function(x, type, as = NULL) { invisible(a) } -expect_code_roundtrip <- function(x, namespace = FALSE, ...) { - expect_equal(eval(x$code(namespace)), x, ...) +expect_code_roundtrip <- function(x) { + code <- x$code() + code_with_ns <- x$code(namespace=TRUE) + + pkg_prefix_pattern <- "^arrow[:][:]" + expect_no_match(as.character(code), pkg_prefix_pattern) + expect_match(as.character(code_with_ns)[1], pkg_prefix_pattern) + + expect_equal(eval(code), x) + expect_equal(eval(code_with_ns), x) } diff --git a/r/tests/testthat/test-schema.R b/r/tests/testthat/test-schema.R index 26bbae931b07e..15342add38fae 100644 --- a/r/tests/testthat/test-schema.R +++ b/r/tests/testthat/test-schema.R @@ -39,10 +39,9 @@ test_that("Schema print method", { }) test_that("Schema$code()", { - schema_obj <- schema(a = int32(), b = struct(c = double(), d = utf8()), e = list_of(binary())) - expect_code_roundtrip(schema_obj) - - expect_no_match(as.character(schema_obj$code()), "arrow::", fixed=TRUE) + expect_code_roundtrip( + schema(a = int32(), b = struct(c = double(), d = utf8()), e = list_of(binary())) + ) skip_if(packageVersion("rlang") < "1") expect_error( @@ -51,13 +50,6 @@ test_that("Schema$code()", { ) }) -test_that("Schema$code(namespace=TRUE)", { - schema_obj <- schema(a = int32(), b = struct(c = double(), d = utf8()), e = list_of(binary())) - expect_code_roundtrip(schema_obj, namespace = TRUE) - - expect_match(as.character(schema_obj$code(TRUE)), "^arrow[:][:]") -}) - test_that("Schema with non-nullable fields", { expect_output( print( diff --git a/r/tests/testthat/test-type.R b/r/tests/testthat/test-type.R index e6aa195a8f347..4f6210c29c1a2 100644 --- a/r/tests/testthat/test-type.R +++ b/r/tests/testthat/test-type.R @@ -343,160 +343,3 @@ test_that("infer_type() infers type for lists starting with NULL - ARROW-17639", list_of(null()) ) }) - - -test_that("code(namespace) works for data types without arguments",{ - # Names encode type aliases. - # No names means the type alias matches the string - type_strs <- c( - "int8", "int16", "int32", "int64", - "uint8", "uint16", "uint32", "uint64", - "halffloat" = "float16", "halffloat", "float" = "float32", "float", "float64", - "bool" = "boolean", "bool", - "utf8", "large_utf8", "binary", "large_binary", "utf8" = "string", - "null", - "date32", "date64", "time32", "time64", "timestamp" - ) - - evaluate_type_str <- function(type_str, type_alias) { - if(type_alias == "") { - type_alias <- type_str - } - type_obj <- eval(call2(type_str, .ns=getPackageName())) - - expect_code_roundtrip(type_obj, info = type_str) - expect_code_roundtrip(type_obj, namespace = TRUE, info = type_str) - - type_code <- as.character(type_obj$code())[1] # Ignore units in time types. - type_code_with_ns <- as.character(type_obj$code(TRUE))[1] # Ignore units in time types. - - expect_equal(type_code, type_alias) - expect_equal(type_code_with_ns, paste0(getPackageName(),"::", type_alias)) - } - purrr::iwalk(type_strs, evaluate_type_str) - -}) - -test_that("code(namespace) works for simple data types with arguments",{ - types_with_args <- list( - # type_str, args, type_alias - list("fixed_size_binary", list(42), "fixed_size_binary"), - list("decimal", list(3, 2), "decimal"), - list("decimal128", list(3, 2), "decimal"), - list("decimal256", list(3, 2), "decimal") - ) - evaluate_type_with_arg <- function(type_with_args) { - type_str <- type_with_args[[1]] - args <- type_with_args[[2]] - type_alias <- type_with_args[[3]] - - type_obj <- eval(call2(type_str, !!!args, .ns=getPackageName())) - - if(type_str == type_alias) { - expect_code_roundtrip(type_obj, info = type_str) - expect_code_roundtrip(type_obj, namespace = TRUE, info = type_str) - } - - type_code <- as.character(type_obj$code()) - type_code_with_ns <- as.character(type_obj$code(TRUE)) - - # test info - build_test_info <- function(test_str, namespace=FALSE) { - glue::glue("`{type_str}` {test_str} (namespace={namespace})") - } - - # type name - expect_equal(type_code[1], type_alias, info=build_test_info("type name")) - expect_equal(type_code_with_ns[1], paste0(getPackageName(), "::", type_alias), info=build_test_info("type name", TRUE)) - - # args - expect_equal(type_code[-1], as.character(unlist(args)), info=build_test_info("args")) - expect_equal(type_code_with_ns[-1], as.character(unlist(args)), info=build_test_info("type name", TRUE)) - } - purrr::walk(types_with_args, evaluate_type_with_arg) -}) - -test_that("code(namespace) works for nested_types",{ - # Nested Types - nested_types <- list( - list("struct", list(foo=int32())), - list("list_of", list(int32())), - list("large_list_of", list(int32())), - list("fixed_size_list_of", list(int32(), 42)) - ) - evaluate_nested_type <- function(nested_type) { - type_str <- nested_type[[1]] - args <- nested_type[[2]] - - type_obj <- eval(call2(type_str, !!!args, .ns=getPackageName())) - - expect_code_roundtrip(type_obj, info = type_str) - expect_code_roundtrip(type_obj, namespace = TRUE, info = type_str) - - type_code <- as.character(type_obj$code()) - type_code_with_ns <- as.character(type_obj$code(TRUE)) - - - # test info - build_test_info <- function(test_str, namespace=FALSE) { - glue::glue("`{type_str}` {test_str} (namespace={namespace})") - } - - # type name - expect_equal(type_code[1], type_str, info=build_test_info("type name")) - expect_equal(type_code_with_ns[1], paste0(getPackageName(), "::", type_str), - info=build_test_info("type name", TRUE)) - - # first arg (also a type) - build_expected_str <- function(namespace=FALSE) { - paste0( - as.character(args[[1]]$code(namespace)), - "()" # The () is kept in arguments. - ) - } - expect_equal(type_code[2], build_expected_str(), info=build_test_info("first arg")) - expect_equal(type_code_with_ns[2], build_expected_str(TRUE), info=build_test_info("first arg", TRUE)) - - # second arg, if exists - if(length(args) == 2) { - second_arg <- as.character(args[[2]]) - expect_equal(type_code[3], second_arg, info=build_test_info("second arg")) - expect_equal(type_code_with_ns[3], second_arg, info=build_test_info("second arg", TRUE)) - } - } - purrr::walk(nested_types, evaluate_nested_type) - -}) - -test_that("code(namespace) works for map_of",{ - - type_str <- "map_of" - args <- list(string(), string()) - type_obj <- eval(call2(type_str, !!!args, .ns=getPackageName())) - type_code <- as.character(type_obj$code()) - type_code_with_ns <- as.character(type_obj$code(TRUE)) - - - # test info - build_test_info <- function(test_str, namespace=FALSE) { - glue::glue("`{type_str}` {test_str} (namespace={namespace})") - } - - # list_of - expect_equal(type_code[1], "list_of", info=build_test_info("list_of")) - expect_equal(type_code_with_ns[1], paste0(getPackageName(), "::", "list_of"), - info=build_test_info("list_of", TRUE)) - - # struct argument - expect_struct_code_matches <- function(struct_code, namespace=FALSE) { - get_code_str <- function(obj) as.character(obj$code(namespace)) - - expect_true(grepl(get_code_str(arrow::struct()), struct_code, fixed = TRUE)) - for(arg in args) { - expect_true(grepl(get_code_str(arg), struct_code, fixed = TRUE)) - } - } - - expect_struct_code_matches(type_code[2]) - expect_struct_code_matches(type_code_with_ns[2]) -}) From 62ec5a0884acdd9535187f7504611739f37357a4 Mon Sep 17 00:00:00 2001 From: orgadish <48453207+orgadish@users.noreply.github.com> Date: Tue, 17 Oct 2023 14:16:33 -0700 Subject: [PATCH 15/17] Address lintr errors. --- r/R/dictionary.R | 2 +- r/R/type.R | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/r/R/dictionary.R b/r/R/dictionary.R index 09cdd3897f8fe..d42a10ad596c7 100644 --- a/r/R/dictionary.R +++ b/r/R/dictionary.R @@ -46,7 +46,7 @@ DictionaryType <- R6Class("DictionaryType", if (isTRUE(self$ordered)) { details$ordered <- TRUE } - call2("dictionary", !!!details, .ns = if(namespace) "arrow") + call2("dictionary", !!!details, .ns = if (namespace) "arrow") } ), active = list( diff --git a/r/R/type.R b/r/R/type.R index b7ce05814dc14..d6db6f146edcd 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -209,7 +209,9 @@ FixedSizeBinary <- R6Class("FixedSizeBinary", inherit = FixedWidthType, public = list( byte_width = function() FixedSizeBinary__byte_width(self), - code = function(namespace = FALSE) call2("fixed_size_binary", byte_width = self$byte_width(), .ns = if (namespace) "arrow") + code = function(namespace = FALSE) { + call2("fixed_size_binary", byte_width = self$byte_width(), .ns = if (namespace) "arrow") + } ) ) @@ -685,7 +687,9 @@ FixedSizeListType <- R6Class("FixedSizeListType", inherit = NestedType, public = list( code = function(namespace = FALSE) { - call2("fixed_size_list_of", self$value_type$code(namespace), list_size = self$list_size, .ns = if (namespace) "arrow") + call2("fixed_size_list_of", self$value_type$code(namespace), + list_size = self$list_size, .ns = if (namespace) "arrow" + ) } ), active = list( From cce444b1e9dcdae3d452f3de715661a98e514dd2 Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Thu, 19 Oct 2023 17:24:26 +0100 Subject: [PATCH 16/17] Add spacing for linter --- r/tests/testthat/helper-roundtrip.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/tests/testthat/helper-roundtrip.R b/r/tests/testthat/helper-roundtrip.R index 4e22737d2b6fc..449a30dd9dbf1 100644 --- a/r/tests/testthat/helper-roundtrip.R +++ b/r/tests/testthat/helper-roundtrip.R @@ -45,7 +45,7 @@ expect_array_roundtrip <- function(x, type, as = NULL) { expect_code_roundtrip <- function(x) { code <- x$code() - code_with_ns <- x$code(namespace=TRUE) + code_with_ns <- x$code(namespace = TRUE) pkg_prefix_pattern <- "^arrow[:][:]" expect_no_match(as.character(code), pkg_prefix_pattern) From 87a272c0773e756fbddd72cc7db83d62aa1994dd Mon Sep 17 00:00:00 2001 From: Nic Crane Date: Thu, 19 Oct 2023 17:27:03 +0100 Subject: [PATCH 17/17] Tiny phrasing change --- r/R/schema.R | 2 +- r/man/Schema-class.Rd | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/r/R/schema.R b/r/R/schema.R index 68a251dc24db8..75623668d9621 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -39,7 +39,7 @@ #' - `$WithMetadata(metadata)`: returns a new `Schema` with the key-value #' `metadata` set. Note that all list elements in `metadata` will be coerced #' to `character`. -#' - `$code(namespace)`: Produces an R call for the schema. Use `namespace=TRUE` to call with `arrow::`. +#' - `$code(namespace)`: returns the R code needed to generate this schema. Use `namespace=TRUE` to call with `arrow::`. #' #' @section Active bindings: #' diff --git a/r/man/Schema-class.Rd b/r/man/Schema-class.Rd index a0c790ca7b705..ecd216af07d78 100644 --- a/r/man/Schema-class.Rd +++ b/r/man/Schema-class.Rd @@ -24,7 +24,7 @@ Many Arrow objects, including \link{Table} and \link{Dataset}, have a \verb{$sch \item \verb{$WithMetadata(metadata)}: returns a new \code{Schema} with the key-value \code{metadata} set. Note that all list elements in \code{metadata} will be coerced to \code{character}. -\item \verb{$code(namespace)}: Produces an R call for the schema. Use \code{namespace=TRUE} to call with \verb{arrow::}. +\item \verb{$code(namespace)}: returns the R code needed to generate this schema. Use \code{namespace=TRUE} to call with \verb{arrow::}. } }