jonkeane commented on a change in pull request #11751:
URL: https://github.com/apache/arrow/pull/11751#discussion_r754733032
##########
File path: r/R/type.R
##########
@@ -109,15 +116,41 @@ Float16 <- R6Class("Float16", inherit = FixedWidthType)
Float32 <- R6Class("Float32", inherit = FixedWidthType)
Float64 <- R6Class("Float64", inherit = FixedWidthType)
Boolean <- R6Class("Boolean", inherit = FixedWidthType)
-Utf8 <- R6Class("Utf8", inherit = DataType)
-LargeUtf8 <- R6Class("LargeUtf8", inherit = DataType)
-Binary <- R6Class("Binary", inherit = DataType)
-FixedSizeBinary <- R6Class("FixedSizeBinary", inherit = FixedWidthType)
-LargeBinary <- R6Class("LargeBinary", inherit = DataType)
+Utf8 <- R6Class("Utf8",
+ inherit = DataType,
+ public = list(
+ code = function() call("utf8")
+ )
+)
+LargeUtf8 <- R6Class("LargeUtf8",
+ inherit = DataType,
+ public = list(
+ code = function() call("large_utf8")
+ )
+)
+Binary <- R6Class("Binary",
+ inherit = DataType,
+ public = list(
+ code = function() call("binary")
+ )
+)
+LargeBinary <- R6Class("LargeBinary",
+ inherit = DataType, public = list(
+ code = function() call("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())
+ )
+)
Review comment:
Would it be beneficial to have some sort of generic method in DataType
and then an attribute in each of these classes that is the quoted string? It's
not _much_ code that's repeated here, but it's a few lines per class
##########
File path: r/tests/testthat/test-data-type.R
##########
@@ -427,3 +427,74 @@ test_that("DataType to C-interface", {
# must clean up the pointer or we leak
delete_arrow_schema(ptr)
})
+
+test_that("DataType$code()", {
+ expect_identical(int8()$code(), call("int8"))
+ expect_identical(uint8()$code(), call("uint8"))
+ expect_identical(int16()$code(), call("int16"))
+ expect_identical(uint16()$code(), call("uint16"))
+ expect_identical(int32()$code(), call("int32"))
+ expect_identical(uint32()$code(), call("uint32"))
+ expect_identical(int64()$code(), call("int64"))
+ expect_identical(uint64()$code(), call("uint64"))
+
+ expect_identical(float16()$code(), call("halffloat"))
+ expect_identical(float32()$code(), call("float"))
+ expect_identical(float64()$code(), call("double"))
+
+ expect_identical(null()$code(), call("null"))
+
+ expect_identical(boolean()$code(), call("bool"))
+ expect_identical(utf8()$code(), call("utf8"))
+ expect_identical(large_utf8()$code(), call("large_utf8"))
+
+ expect_identical(binary()$code(), call("binary"))
+ expect_identical(large_binary()$code(), call("large_binary"))
+ expect_identical(fixed_size_binary(byte_width = 91)$code(),
call("fixed_size_binary", byte_width = 91L))
+
+ expect_identical(date32()$code(), call("date32"))
+ expect_identical(date64()$code(), call("date64"))
+
+ expect_identical(time32()$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "ms")$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "s")$code(), call("time32", unit = "s"))
+
+ expect_identical(time64()$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "ns")$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "us")$code(), call("time64", unit = "us"))
+
+ expect_identical(timestamp()$code(), call("timestamp", unit = "s"))
+ expect_identical(timestamp(unit = "s" )$code(), call("timestamp", unit =
"s"))
+ expect_identical(timestamp(unit = "ms")$code(), call("timestamp", unit =
"ms"))
+ expect_identical(timestamp(unit = "ns")$code(), call("timestamp", unit =
"ns"))
+ expect_identical(timestamp(unit = "us")$code(), call("timestamp", unit =
"us"))
+
+ expect_identical(timestamp(unit = "s" , timezone = "CET")$code(),
call("timestamp", unit = "s" , timezone = "CET"))
+ expect_identical(timestamp(unit = "ms", timezone = "CET")$code(),
call("timestamp", unit = "ms", timezone = "CET"))
+ expect_identical(timestamp(unit = "ns", timezone = "CET")$code(),
call("timestamp", unit = "ns", timezone = "CET"))
+ expect_identical(timestamp(unit = "us", timezone = "CET")$code(),
call("timestamp", unit = "us", timezone = "CET"))
+
+ expect_identical(decimal(precision = 3, scale = 5)$code(), call("decimal",
precision = 3L, scale = 5L))
+
+ expect_identical(
+ struct(a = int32(), b = struct(c = list_of(utf8())), d = float64())$code(),
+ quote(struct(a = int32(), b = struct(c = list_of(utf8())), d = double()))
+ )
+
+ expect_identical(list_of(int32())$code(), quote(list_of(int32())))
+ expect_identical(large_list_of(int32())$code(),
quote(large_list_of(int32())))
+ expect_identical(fixed_size_list_of(int32(), list_size = 7L)$code(),
quote(fixed_size_list_of(int32(), list_size = 7L)))
+
+ skip("until rlang 1.0")
+ expect_snapshot({
+ (expect_error(
+ DayTimeInterval__initialize()$code()
+ ))
+ (expect_error(
+ struct(a = DayTimeInterval__initialize())$code()
+ ))
+ })
+
+})
+
+
Review comment:
```suggestion
```
##########
File path: r/tests/testthat/test-data-type.R
##########
@@ -427,3 +427,74 @@ test_that("DataType to C-interface", {
# must clean up the pointer or we leak
delete_arrow_schema(ptr)
})
+
+test_that("DataType$code()", {
+ expect_identical(int8()$code(), call("int8"))
+ expect_identical(uint8()$code(), call("uint8"))
+ expect_identical(int16()$code(), call("int16"))
+ expect_identical(uint16()$code(), call("uint16"))
+ expect_identical(int32()$code(), call("int32"))
+ expect_identical(uint32()$code(), call("uint32"))
+ expect_identical(int64()$code(), call("int64"))
+ expect_identical(uint64()$code(), call("uint64"))
+
+ expect_identical(float16()$code(), call("halffloat"))
+ expect_identical(float32()$code(), call("float"))
+ expect_identical(float64()$code(), call("double"))
+
+ expect_identical(null()$code(), call("null"))
+
+ expect_identical(boolean()$code(), call("bool"))
+ expect_identical(utf8()$code(), call("utf8"))
+ expect_identical(large_utf8()$code(), call("large_utf8"))
+
+ expect_identical(binary()$code(), call("binary"))
+ expect_identical(large_binary()$code(), call("large_binary"))
+ expect_identical(fixed_size_binary(byte_width = 91)$code(),
call("fixed_size_binary", byte_width = 91L))
+
+ expect_identical(date32()$code(), call("date32"))
+ expect_identical(date64()$code(), call("date64"))
+
+ expect_identical(time32()$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "ms")$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "s")$code(), call("time32", unit = "s"))
+
+ expect_identical(time64()$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "ns")$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "us")$code(), call("time64", unit = "us"))
+
+ expect_identical(timestamp()$code(), call("timestamp", unit = "s"))
+ expect_identical(timestamp(unit = "s" )$code(), call("timestamp", unit =
"s"))
+ expect_identical(timestamp(unit = "ms")$code(), call("timestamp", unit =
"ms"))
+ expect_identical(timestamp(unit = "ns")$code(), call("timestamp", unit =
"ns"))
+ expect_identical(timestamp(unit = "us")$code(), call("timestamp", unit =
"us"))
+
+ expect_identical(timestamp(unit = "s" , timezone = "CET")$code(),
call("timestamp", unit = "s" , timezone = "CET"))
Review comment:
```suggestion
expect_identical(timestamp(unit = "s", timezone = "CET")$code(),
call("timestamp", unit = "s", timezone = "CET"))
```
##########
File path: r/R/type.R
##########
@@ -109,15 +116,41 @@ Float16 <- R6Class("Float16", inherit = FixedWidthType)
Float32 <- R6Class("Float32", inherit = FixedWidthType)
Float64 <- R6Class("Float64", inherit = FixedWidthType)
Boolean <- R6Class("Boolean", inherit = FixedWidthType)
-Utf8 <- R6Class("Utf8", inherit = DataType)
-LargeUtf8 <- R6Class("LargeUtf8", inherit = DataType)
-Binary <- R6Class("Binary", inherit = DataType)
-FixedSizeBinary <- R6Class("FixedSizeBinary", inherit = FixedWidthType)
-LargeBinary <- R6Class("LargeBinary", inherit = DataType)
+Utf8 <- R6Class("Utf8",
+ inherit = DataType,
+ public = list(
+ code = function() call("utf8")
+ )
+)
+LargeUtf8 <- R6Class("LargeUtf8",
+ inherit = DataType,
+ public = list(
+ code = function() call("large_utf8")
+ )
+)
+Binary <- R6Class("Binary",
+ inherit = DataType,
+ public = list(
+ code = function() call("binary")
+ )
+)
+LargeBinary <- R6Class("LargeBinary",
+ inherit = DataType, public = list(
+ code = function() call("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())
+ )
+)
Review comment:
Oh, actually I see below we have one for all the non-exceptionally named
ones. This is probably fine since it's a limited number of classes where we
need to do it
##########
File path: r/tests/testthat/test-data-type.R
##########
@@ -427,3 +427,74 @@ test_that("DataType to C-interface", {
# must clean up the pointer or we leak
delete_arrow_schema(ptr)
})
+
+test_that("DataType$code()", {
+ expect_identical(int8()$code(), call("int8"))
+ expect_identical(uint8()$code(), call("uint8"))
+ expect_identical(int16()$code(), call("int16"))
+ expect_identical(uint16()$code(), call("uint16"))
+ expect_identical(int32()$code(), call("int32"))
+ expect_identical(uint32()$code(), call("uint32"))
+ expect_identical(int64()$code(), call("int64"))
+ expect_identical(uint64()$code(), call("uint64"))
+
+ expect_identical(float16()$code(), call("halffloat"))
+ expect_identical(float32()$code(), call("float"))
+ expect_identical(float64()$code(), call("double"))
+
+ expect_identical(null()$code(), call("null"))
+
+ expect_identical(boolean()$code(), call("bool"))
+ expect_identical(utf8()$code(), call("utf8"))
+ expect_identical(large_utf8()$code(), call("large_utf8"))
+
+ expect_identical(binary()$code(), call("binary"))
+ expect_identical(large_binary()$code(), call("large_binary"))
+ expect_identical(fixed_size_binary(byte_width = 91)$code(),
call("fixed_size_binary", byte_width = 91L))
+
+ expect_identical(date32()$code(), call("date32"))
+ expect_identical(date64()$code(), call("date64"))
+
+ expect_identical(time32()$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "ms")$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "s")$code(), call("time32", unit = "s"))
+
+ expect_identical(time64()$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "ns")$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "us")$code(), call("time64", unit = "us"))
+
+ expect_identical(timestamp()$code(), call("timestamp", unit = "s"))
+ expect_identical(timestamp(unit = "s" )$code(), call("timestamp", unit =
"s"))
Review comment:
```suggestion
expect_identical(timestamp(unit = "s")$code(), call("timestamp", unit =
"s"))
```
##########
File path: r/R/schema.R
##########
@@ -121,7 +121,25 @@ Schema <- R6Class("Schema",
Equals = function(other, check_metadata = FALSE, ...) {
inherits(other, "Schema") && Schema__Equals(self, other,
isTRUE(check_metadata))
},
- export_to_c = function(ptr) ExportSchema(self, ptr)
+ export_to_c = function(ptr) ExportSchema(self, ptr),
+ code = function() {
+ names <- self$names
+ codes <- map2(names, self$fields, function(name, field){
Review comment:
```suggestion
codes <- map2(names, self$fields, function(name, field) {
```
##########
File path: r/tests/testthat/test-data-type.R
##########
@@ -427,3 +427,74 @@ test_that("DataType to C-interface", {
# must clean up the pointer or we leak
delete_arrow_schema(ptr)
})
+
+test_that("DataType$code()", {
+ expect_identical(int8()$code(), call("int8"))
+ expect_identical(uint8()$code(), call("uint8"))
+ expect_identical(int16()$code(), call("int16"))
+ expect_identical(uint16()$code(), call("uint16"))
+ expect_identical(int32()$code(), call("int32"))
+ expect_identical(uint32()$code(), call("uint32"))
+ expect_identical(int64()$code(), call("int64"))
+ expect_identical(uint64()$code(), call("uint64"))
+
+ expect_identical(float16()$code(), call("halffloat"))
+ expect_identical(float32()$code(), call("float"))
+ expect_identical(float64()$code(), call("double"))
+
+ expect_identical(null()$code(), call("null"))
+
+ expect_identical(boolean()$code(), call("bool"))
+ expect_identical(utf8()$code(), call("utf8"))
+ expect_identical(large_utf8()$code(), call("large_utf8"))
+
+ expect_identical(binary()$code(), call("binary"))
+ expect_identical(large_binary()$code(), call("large_binary"))
+ expect_identical(fixed_size_binary(byte_width = 91)$code(),
call("fixed_size_binary", byte_width = 91L))
+
+ expect_identical(date32()$code(), call("date32"))
+ expect_identical(date64()$code(), call("date64"))
+
+ expect_identical(time32()$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "ms")$code(), call("time32", unit = "ms"))
+ expect_identical(time32(unit = "s")$code(), call("time32", unit = "s"))
+
+ expect_identical(time64()$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "ns")$code(), call("time64", unit = "ns"))
+ expect_identical(time64(unit = "us")$code(), call("time64", unit = "us"))
+
+ expect_identical(timestamp()$code(), call("timestamp", unit = "s"))
+ expect_identical(timestamp(unit = "s" )$code(), call("timestamp", unit =
"s"))
+ expect_identical(timestamp(unit = "ms")$code(), call("timestamp", unit =
"ms"))
+ expect_identical(timestamp(unit = "ns")$code(), call("timestamp", unit =
"ns"))
+ expect_identical(timestamp(unit = "us")$code(), call("timestamp", unit =
"us"))
+
+ expect_identical(timestamp(unit = "s" , timezone = "CET")$code(),
call("timestamp", unit = "s" , timezone = "CET"))
+ expect_identical(timestamp(unit = "ms", timezone = "CET")$code(),
call("timestamp", unit = "ms", timezone = "CET"))
+ expect_identical(timestamp(unit = "ns", timezone = "CET")$code(),
call("timestamp", unit = "ns", timezone = "CET"))
+ expect_identical(timestamp(unit = "us", timezone = "CET")$code(),
call("timestamp", unit = "us", timezone = "CET"))
+
+ expect_identical(decimal(precision = 3, scale = 5)$code(), call("decimal",
precision = 3L, scale = 5L))
+
+ expect_identical(
+ struct(a = int32(), b = struct(c = list_of(utf8())), d = float64())$code(),
+ quote(struct(a = int32(), b = struct(c = list_of(utf8())), d = double()))
+ )
+
+ expect_identical(list_of(int32())$code(), quote(list_of(int32())))
+ expect_identical(large_list_of(int32())$code(),
quote(large_list_of(int32())))
+ expect_identical(fixed_size_list_of(int32(), list_size = 7L)$code(),
quote(fixed_size_list_of(int32(), list_size = 7L)))
Review comment:
```suggestion
expect_identical(
fixed_size_list_of(int32(), list_size = 7L)$code(),
quote(fixed_size_list_of(int32(), list_size = 7L))
)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
