jonkeane commented on a change in pull request #11790:
URL: https://github.com/apache/arrow/pull/11790#discussion_r759596467
##########
File path: r/tests/testthat/test-data-type.R
##########
@@ -385,16 +385,20 @@ test_that("DictionaryType validation", {
})
test_that("decimal type and validation", {
- expect_error(decimal())
+ expect_r6_class(decimal(4, 2), "Decimal128Type")
+
expect_error(decimal("four"), '"precision" must be an integer')
- expect_error(decimal(4))
expect_error(decimal(4, "two"), '"scale" must be an integer')
expect_error(decimal(NA, 2), '"precision" must be an integer')
- expect_error(decimal(0, 2), "Invalid: Decimal precision out of range [1,
38]: 0", fixed = TRUE)
- expect_error(decimal(100, 2), "Invalid: Decimal precision out of range [1,
38]: 100", fixed = TRUE)
expect_error(decimal(4, NA), '"scale" must be an integer')
- expect_r6_class(decimal(4, 2), "Decimal128Type")
+ expect_r6_class(decimal128(4, 2), "Decimal128Type")
+ expect_identical(class(decimal(2, 4)), class(decimal128(2, 4)))
Review comment:
We might want to include a comment above this to make super explicit
that `decimal()` is an alias for `decimal128()`.
##########
File path: r/tests/testthat/test-dplyr-funcs-type.R
##########
@@ -208,32 +208,42 @@ test_that("type checks with is() giving Arrow types", {
Table$create(
i32 = Array$create(1, int32()),
dec = Array$create(pi)$cast(decimal(3, 2)),
+ dec128 = Array$create(pi)$cast(decimal128(3, 2)),
f64 = Array$create(1.1, float64()),
str = Array$create("a", arrow::string())
) %>% transmute(
i32_is_i32 = is(i32, int32()),
i32_is_dec = is(i32, decimal(3, 2)),
+ i32_is_dec128 = is(i32, decimal128(3, 2)),
i32_is_i64 = is(i32, float64()),
i32_is_str = is(i32, arrow::string()),
dec_is_i32 = is(dec, int32()),
dec_is_dec = is(dec, decimal(3, 2)),
+ dec_is_dec128 = is(dec, decimal128(3, 2)),
dec_is_i64 = is(dec, float64()),
dec_is_str = is(dec, arrow::string()),
+ dec128_is_i32 = is(dec128, int32()),
+ dec128_is_dec128 = is(dec128, decimal128(3, 2)),
+ dec128_is_i64 = is(dec128, float64()),
+ dec128_is_str = is(dec128, arrow::string()),
f64_is_i32 = is(f64, int32()),
f64_is_dec = is(f64, decimal(3, 2)),
+ f64_is_dec128 = is(f64, decimal128(3, 2)),
f64_is_i64 = is(f64, float64()),
f64_is_str = is(f64, arrow::string()),
str_is_i32 = is(str, int32()),
str_is_dec = is(str, decimal(3, 2)),
+ str_is_dec128 = is(str, decimal128(3, 2)),
str_is_i64 = is(str, float64()),
str_is_str = is(str, arrow::string())
Review comment:
I'm fine keeping both dec* and dec128* here, but technically they are
all testing the same thing, right? If we were to clean this up, we should use
dec128 for each of these (since we test elsewhere that `decimal(x, y) ==
decimal128(x, y)`
--
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]
