Skip to content

Commit

Permalink
Add extra validation when .by is supplied and a group data frame (r…
Browse files Browse the repository at this point in the history
…epeat the error message from mutate() for a better stack trace.
  • Loading branch information
olivroy authored and DavisVaughan committed Aug 27, 2024
1 parent f128f26 commit c346ccb
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 0 deletions.
5 changes: 5 additions & 0 deletions R/fill.R
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ fill.data.frame <- function(data, ..., .direction = c("down", "up", "downup", "u
}

if (dplyr::is_grouped_df(data)) {
if (length(by) > 0) {
cli::cli_abort(c(
"Can't supply {.arg .by} when {.arg data} is a grouped data frame."
))
}
dplyr::mutate(data, dplyr::across(any_of(vars), .fns = fn))
} else {
dplyr::mutate(data, dplyr::across(any_of(vars), .fns = fn), .by = all_of(by))
Expand Down
5 changes: 5 additions & 0 deletions tests/testthat/_snaps/fill.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,9 @@
Error in `fill()`:
! Can't select columns that don't exist.
x Column `z` doesn't exist.
Code
gr_df %>% fill(y, .by = x)
Condition
Error in `fill()`:
! Can't supply `.by` when `data` is a grouped data frame.

2 changes: 2 additions & 0 deletions tests/testthat/test-fill.R
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,9 @@ test_that("validates its inputs", {

test_that("fill works with .by", {
df <- tibble(x = c(1, 1, 2), y = c(1, NA, NA))
gr_df <- dplyr::group_by(df, x)
expect_snapshot(error = TRUE, {
df %>% fill(y, .by = z)
gr_df %>% fill(y, .by = x)
})
})

0 comments on commit c346ccb

Please sign in to comment.