paleolimbot commented on a change in pull request #11133:
URL: https://github.com/apache/arrow/pull/11133#discussion_r707511303



##########
File path: r/tests/testthat/test-dplyr.R
##########
@@ -1035,10 +1035,20 @@ test_that("log functions", {
     df
   )
 
-  expect_error(
-    nse_funcs$log(Expression$scalar(x), base = 5),
-    "`base` values other than exp(1), 2 and 10 not supported by Arrow",
-    fixed = TRUE
+  expect_dplyr_equal(
+    input %>%
+      mutate(y = log(x, base = 5)) %>%
+      collect(),
+    df
+  )
+
+  expect_dplyr_equal(
+    input %>%
+      # suppress 'NaNs produced' warning on the first row of df
+      # that evaluates to NaN (R raises warning but Arrow does not)
+      suppressWarnings(mutate(., y = log(x, base = x))) %>%

Review comment:
       I added a test in 
https://github.com/apache/arrow/pull/11133/commits/812f941a7720d3e73236d032e8cbf54db171353e
 that ensures `NaN` for `log(1, base = 1)` and `Inf` for `log(10, base = 1)` in 
both dplyr and arrow. Is that test reasonable or should it be 
rewritten/clarified?




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