nealrichardson commented on a change in pull request #12432:
URL: https://github.com/apache/arrow/pull/12432#discussion_r812406136



##########
File path: r/R/arrow-datum.R
##########
@@ -70,13 +70,50 @@ as.vector.ArrowDatum <- function(x, mode) {
 
 #' @export
 Ops.ArrowDatum <- function(e1, e2) {
-  if (.Generic == "!") {
-    eval_array_expression(.Generic, e1)
-  } else if (.Generic %in% names(.array_function_map)) {
-    eval_array_expression(.Generic, e1, e2)
-  } else {
-    stop(paste0("Unsupported operation on `", class(e1)[1L], "` : "), 
.Generic, call. = FALSE)
+  if (missing(e2)) {
+    switch(
+      .Generic,
+      "!" = return(eval_array_expression(.Generic, e1)),
+      "+" =, "-" = return(eval_array_expression(.Generic, 0L, e1)),
+    )
   }
+
+  switch(
+    .Generic,
+    "+" =, "-" =, "*" =, "/" =, "^" =, "%%" =, "%/%" =,
+    "==" =, "!=" =, "<" =, "<=" =, ">=" =, ">" =, "&" =, "|" = {
+      eval_array_expression(.Generic, e1, e2)
+    },
+    stop(paste0("Unsupported operation on `", class(e1)[1L], "` : "), 
.Generic, call. = FALSE)
+  )
+}
+
+#' @export
+Math.ArrowDatum <- function(x, ..., base = exp(1), digits = 0) {
+  switch(
+    .Generic,
+    abs =, sign =,
+    floor =, ceiling =, trunc =,
+    acos =, asin =, atan =,
+    cos =, sin =, tan = {
+      eval_array_expression(.Generic, x)
+    },
+    log = eval_array_expression("logb_checked", x, base),
+    log10 = eval_array_expression("log10_checked", x),
+    round = eval_array_expression(
+      "round",
+      x,
+      options = list(ndigits = digits, round_mode = RoundMode$HALF_TO_EVEN)
+    ),
+    sqrt =, exp =,
+    signif =, expm1 =, log1p =,
+    cospi =, sinpi =, tanpi =,
+    cosh =, sinh =, tanh =,
+    acosh =, asinh =, atanh =,
+    lgamma =, gamma =, digamma =, trigamma =,
+    cumsum =, cumprod =, cummax =, cumin =,

Review comment:
       ```suggestion
       cumsum =, cumprod =, cummax =, cummin =,
   ```

##########
File path: r/R/arrow-datum.R
##########
@@ -70,13 +70,50 @@ as.vector.ArrowDatum <- function(x, mode) {
 
 #' @export
 Ops.ArrowDatum <- function(e1, e2) {
-  if (.Generic == "!") {
-    eval_array_expression(.Generic, e1)
-  } else if (.Generic %in% names(.array_function_map)) {
-    eval_array_expression(.Generic, e1, e2)
-  } else {
-    stop(paste0("Unsupported operation on `", class(e1)[1L], "` : "), 
.Generic, call. = FALSE)
+  if (missing(e2)) {
+    switch(
+      .Generic,
+      "!" = return(eval_array_expression(.Generic, e1)),
+      "+" =, "-" = return(eval_array_expression(.Generic, 0L, e1)),
+    )
   }
+
+  switch(
+    .Generic,
+    "+" =, "-" =, "*" =, "/" =, "^" =, "%%" =, "%/%" =,
+    "==" =, "!=" =, "<" =, "<=" =, ">=" =, ">" =, "&" =, "|" = {
+      eval_array_expression(.Generic, e1, e2)
+    },
+    stop(paste0("Unsupported operation on `", class(e1)[1L], "` : "), 
.Generic, call. = FALSE)
+  )
+}
+
+#' @export
+Math.ArrowDatum <- function(x, ..., base = exp(1), digits = 0) {
+  switch(
+    .Generic,
+    abs =, sign =,
+    floor =, ceiling =, trunc =,
+    acos =, asin =, atan =,
+    cos =, sin =, tan = {
+      eval_array_expression(.Generic, x)
+    },
+    log = eval_array_expression("logb_checked", x, base),
+    log10 = eval_array_expression("log10_checked", x),
+    round = eval_array_expression(
+      "round",
+      x,
+      options = list(ndigits = digits, round_mode = RoundMode$HALF_TO_EVEN)
+    ),
+    sqrt =, exp =,

Review comment:
       Can't we do these since we have support for `^`?

##########
File path: r/tests/testthat/test-compute-arith.R
##########
@@ -133,3 +133,38 @@ test_that("Dates casting", {
   # Error: NotImplemented: Function add_checked has no kernel matching input 
types (array[date32[day]], scalar[double])
   expect_equal(a + 2, Array$create(c((Sys.Date() + 1:4) + 2), NA_integer_))
 })
+
+test_that("Math group generics work on Array objects", {
+  expect_equal(abs(Array$create(c(-1L, 1L))), Array$create(c(1L, 1L)))
+  expect_equal(
+    sign(Array$create(c(-1L, 1L))),

Review comment:
       Just to make clear that it's not the identity function
   
   ```suggestion
       sign(Array$create(c(-5L, 2L))),
   ```

##########
File path: r/R/arrow-datum.R
##########
@@ -70,13 +70,50 @@ as.vector.ArrowDatum <- function(x, mode) {
 
 #' @export
 Ops.ArrowDatum <- function(e1, e2) {

Review comment:
       Why the changes to Ops.ArrowDatum? I don't see any test changes, and I 
don't follow how this change is an improvement.




-- 
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]


Reply via email to