paleolimbot commented on code in PR #15211:
URL: https://github.com/apache/arrow/pull/15211#discussion_r1067012468


##########
r/tests/testthat/test-Array.R:
##########
@@ -1321,3 +1321,19 @@ test_that("Array to C-interface", {
   delete_arrow_schema(schema_ptr)
   delete_arrow_array(array_ptr)
 })
+
+test_that("direct creation of Decimal Arrays (ARROW-11631)", {
+
+  decimal_array <- Array$create(1, type = decimal128(10, 2))
+  decimal_array2 <- Array$create(1, type = decimal256(10, 2))
+
+  expect_equal(

Review Comment:
   Because there's a branch for ALTREP, maybe `decimal_array2` could be 
`Array$create(1:10, type = decimal128(10, 2))`?



##########
r/tests/testthat/test-Array.R:
##########
@@ -1321,3 +1321,19 @@ test_that("Array to C-interface", {
   delete_arrow_schema(schema_ptr)
   delete_arrow_array(array_ptr)
 })
+
+test_that("direct creation of Decimal Arrays (ARROW-11631)", {
+
+  decimal_array <- Array$create(1, type = decimal128(10, 2))
+  decimal_array2 <- Array$create(1, type = decimal256(10, 2))

Review Comment:
   Could the test Array include an `NA` and something whose precision might get 
truncated (i.e, maybe `Array$create(c(1, 1 / 3, NA), type = decimal128(10, 
2))`)? (The `NA` is to get full test coverage since there's a branch for nulls 
in the C++; the truncation is to check that `type` gets passed through 
properly).
   
   The purpose of `decimal_array2` isn't clear to me here...am I missing 
something?



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to