[GitHub] [arrow] nealrichardson commented on a change in pull request #10689: ARROW-13280: [R] Bindings for log and trig functions

[GitBox <gi...@apache.org>]

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



##########
File path: r/R/expression.R
##########
@@ -43,7 +43,17 @@
   "yday" = "day_of_year",
   "hour" = "hour",
   # second is defined in dplyr-functions.R
-  "minute" = "minute"
+  "minute" = "minute",
+  "log" = "ln_checked",

Review comment:
       Now that these exist, we should define the `Math` group generic for Arrow objects, like we have for `Ops`. I made https://issues.apache.org/jira/browse/ARROW-13337 for that.

##########
File path: r/R/expression.R
##########
@@ -43,7 +43,17 @@
   "yday" = "day_of_year",
   "hour" = "hour",
   # second is defined in dplyr-functions.R
-  "minute" = "minute"
+  "minute" = "minute",
+  "log" = "ln_checked",

Review comment:
       Hmm, in R, `log` is not unary: 
   
   ```
   log <- function (x, base = exp(1))
   ```
   
   So we should handle that correctly. If arrow doesn't support general `log_baseN`, (1) make sure there's a JIRA for that, and (2) map the cases that do exist (e, 10, 2, etc.) and for now error "not supported" for any other value of base.




-- 
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-unsubscribe@arrow.apache.org

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