Skip to content

Commit e2d611d

Browse files
authored
Correctly support .data pronoun within across (#1685)
Fixes #1520
1 parent 5a16f0b commit e2d611d

File tree

4 files changed

+72
-15
lines changed

4 files changed

+72
-15
lines changed

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# dbplyr (development version)
22

3+
* `.data$col`, `.data[[col]]`, `.env$var`, and `.env$[[var]]` now work correctly inside `across()` (#1520).
34
* New `.sql` pronoun makes it a little easier to use known SQL functions in packages, requiring only `@importFrom dbplyr .sql` (#1117).
45
* `join_by(between())` now correctly handles column renames (#1572).
56
* SQL Server uses `DATEDIFF_BIG` instead of `DATEDIFF` to work regardless of data size (@edward-burn, #1666).

R/tidyeval-across.R

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ across_fun <- function(fun, env, dots, fn) {
210210
)
211211
}
212212

213-
partial_eval_prepare_fun(f_rhs(fun), c(".", ".x"))
213+
partial_eval_prepare_fun(f_rhs(fun), c(".", ".x"), env)
214214
} else if (is_call(fun, "function")) {
215215
fun <- eval(fun, env)
216216
partial_eval_fun(fun, env, fn)
@@ -235,10 +235,12 @@ partial_eval_fun <- function(fun, env, fn) {
235235
}
236236
args <- fn_fmls_names(fun)
237237

238-
partial_eval_prepare_fun(body[[2]], args[[1]])
238+
partial_eval_prepare_fun(body[[2]], args[[1]], fn_env(fun))
239239
}
240240

241-
partial_eval_prepare_fun <- function(call, sym) {
241+
partial_eval_prepare_fun <- function(call, sym, env) {
242+
# First resolve any .data/.env pronouns before symbol replacement
243+
call <- resolve_mask_pronouns(call, env)
242244
call <- replace_sym(call, sym, replace = quote(!!.x))
243245
call <- replace_call(call, replace = quote(!!.cur_col))
244246
function(x, .cur_col) {
@@ -249,6 +251,31 @@ partial_eval_prepare_fun <- function(call, sym) {
249251
}
250252
}
251253

254+
resolve_mask_pronouns <- function(call, env) {
255+
if (is_mask_pronoun(call)) {
256+
var <- call[[3]]
257+
258+
if (is_symbol(call[[2]], ".data")) {
259+
if (is_call(call, "[[")) {
260+
sym(eval(var, env))
261+
} else {
262+
var
263+
}
264+
} else {
265+
if (is_call(call, "[[")) {
266+
env_get(env, var)
267+
} else {
268+
env_get(env, as.character(var))
269+
}
270+
}
271+
} else if (is_call(call)) {
272+
call[] <- lapply(call, resolve_mask_pronouns, env = env)
273+
call
274+
} else {
275+
call
276+
}
277+
}
278+
252279
across_setup <- function(data, call, env, allow_rename, fn, error_call) {
253280
grps <- group_vars(data)
254281
tbl <- ungroup(data)

tests/testthat/_snaps/tidyeval-across.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,15 @@
8080
SELECT SUM(`a`) AS `a`, SUM(`b`) AS `b`
8181
FROM `df`
8282

83+
# lambdas in across() can use columns
84+
85+
Code
86+
show_query(db_across)
87+
Output
88+
<SQL>
89+
SELECT `x` / `y` AS `x`, `y` / `y` AS `y`, `z` / `y` AS `z`
90+
FROM `across`
91+
8392
# across() errors if named
8493

8594
Code

tests/testthat/test-tidyeval-across.R

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -274,28 +274,48 @@ test_that("across() uses environment from the current quosure (dplyr#5460)", {
274274
})
275275

276276
test_that("lambdas in across() can use columns", {
277-
lf <- lazy_frame(x = 2, y = 4, z = 8)
277+
db <- local_memdb_frame("across", x = 2, y = 4, z = 8)
278278

279279
expect_equal(
280-
partial_eval_dots(lf, across(everything(), ~ .x / y)),
280+
partial_eval_dots(db, across(everything(), ~ .x / y)),
281281
list(
282282
x = quo(x / y),
283283
y = quo(y / y),
284284
z = quo(z / y)
285285
)
286286
)
287287

288-
skip("not yet correctly supported")
289-
# dplyr uses the old value of `y` for division
290-
df <- tibble(x = 2, y = 4, z = 8)
291-
df |> mutate(across(everything(), ~ .x / .data$y))
292-
# so this is the equivalent
293-
df |> mutate(data.frame(x = x / y, y = y / y, z = z / y))
294-
# dbplyr uses the new value of `y`
295-
lf |> mutate(across(everything(), ~ .x / .data$y))
288+
db_across <- db |> mutate(across(everything(), ~ .x / y))
289+
expect_snapshot(db_across |> show_query())
296290

297-
# so this is the dbplyr equivalent
298-
df |> mutate(x = x / y, y = y / y, z = z / y)
291+
# z should be 2 because the value of .data$y is only transformed
292+
# _after_ across() is complete, the same as
293+
# db |> collect() |> mutate(across(everything(), ~ .x / .data$y))
294+
expect_equal(collect(db_across), tibble(x = 0.5, y = 1, z = 2))
295+
})
296+
297+
test_that("can use .data and .env pronouns(#1520)", {
298+
lf <- lazy_frame(x = 1, y = 2)
299+
300+
my_col <- "y"
301+
expect_equal(
302+
capture_across(lf, across(x:y, !!quo(~ .x / .data$y))),
303+
exprs(x = x / y, y = y / y)
304+
)
305+
expect_equal(
306+
capture_across(lf, across(x:y, !!quo(~ .x / .data[[my_col]]))),
307+
exprs(x = x / y, y = y / y)
308+
)
309+
310+
y <- 10
311+
expect_equal(
312+
capture_across(lf, across(x:y, !!quo(~ .x / .env$y))),
313+
exprs(x = x / 10, y = y / 10)
314+
)
315+
expect_equal(
316+
capture_across(lf, across(x:y, !!quo(~ .x / .env[["y"]]))),
317+
exprs(x = x / 10, y = y / 10)
318+
)
299319
})
300320

301321
test_that("can pass quosure through `across()`", {

0 commit comments

Comments
 (0)