paleolimbot commented on code in PR #13985:
URL: https://github.com/apache/arrow/pull/13985#discussion_r964294898
##########
r/tests/testthat/test-dplyr-funcs-datetime.R:
##########
@@ -183,8 +183,8 @@ test_that("strptime", {
expect_equal(
test_df %>%
arrow_table() %>%
- mutate(x = strptime(x, format = fmt)) %>%
Review Comment:
Did you make any changes to this file? (All I see are whitespace changes)
##########
r/R/expression.R:
##########
@@ -210,21 +213,20 @@ build_expr <- function(FUN,
}
if (FUN == "%in%") {
# Special-case %in%, which is different from the Array function name
+ value_set <- Array$create(args[[2]])
+ try(
+ value_set <- cast_or_parse(value_set, args[[1]]$type()),
+ silent = TRUE
+ )
+
expr <- Expression$create("is_in", args[[1]],
options = list(
- # If args[[2]] is already an Arrow object (like a scalar),
- # this wouldn't work
- value_set = Array$create(args[[2]]),
+ value_set = value_set,
skip_nulls = TRUE
)
)
} else {
- args <- lapply(args, function(x) {
- if (!inherits(x, "Expression")) {
- x <- Expression$scalar(x)
- }
- x
- })
+ args <- wrap_scalars(args, FUN)
Review Comment:
Do you want to whitelist the functions this applies to? (Or maybe you
already do this and I'm not reading this correctly?) This logic is awesome and
very appropriate for most math functions but I wonder if there are some compute
functions (maybe `binary_repeat`) that will stop working when used with
`build_expr()`. I think that user-defined functions also generate their
bindings through `build_expr()` (although they don't have to).
--
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]
