Skip to content

Commit

Permalink
Merge pull request #540 from yjunechoe/has_columns-character-bugfix
Browse files Browse the repository at this point in the history
bugfix `has_columns()` passing character vector of non-existing columns
  • Loading branch information
yjunechoe authored Jun 15, 2024
2 parents c54b035 + 9c2606a commit 21b3228
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
11 changes: 7 additions & 4 deletions R/has_columns.R
Original file line number Diff line number Diff line change
Expand Up @@ -206,20 +206,23 @@ has_columns <- function(
allow_empty = FALSE, call = .call),
error = function(cnd) cnd
)
## Check for error from {tidyselect}
## If error from {tidyselect}, counts as no selection
if (rlang::is_error(columns)) {
cnd <- columns
# Rethrow error if genuine evaluation error
if (inherits(cnd, "resolve_eval_err")) {
rlang::cnd_signal(cnd)
}
# 0-vector if "column not found" or "0 columns" error
# Return length-0 vector if "column not found" or "0 columns" error
return(character(0L))
} else {
## If columns succesfully resolved to character, return only existing ones
return(intersect(columns, colnames(x)))
}
# vector of selections if successful
return(columns)
}

# A list of columns (character vector) selected by elements of `columns`
# - Ex: `c(a, b:c)` becomes `list("a", c("b", "c"))` if data has those columns
columns_list <- lapply(column_quos, has_column)
all(lengths(columns_list) > 0L)

Expand Down
11 changes: 9 additions & 2 deletions tests/testthat/test-has_columns.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@ test_that("the `has_columns()` function works when used directly with data", {
expect_true(small_table %>% has_columns(c(vars(a, b), vars(c))))

# Expect FALSE when *any* of the given column names is absent
expect_false(small_table %>% has_columns(vars(a, h)))
expect_false(small_table %>% has_columns(vars(h, j)))
expect_false(small_table %>% has_columns(vars(z)))
expect_false(small_table %>% has_columns("z"))
expect_false(small_table %>% has_columns(vars(a, z)))
expect_false(small_table %>% has_columns(vars(z, zz)))
expect_false(small_table %>% has_columns(c("a", "z")))
expect_false(small_table %>% has_columns(c("z", "zz")))

# Expect that using inputs that are not tabular result in errors
expect_error(has_columns(list(a = "2"), "a"))
Expand All @@ -34,6 +38,9 @@ test_that("the `has_columns()` function works with tidyselect", {
expect_false(small_table %>% has_columns(last_col() + 1))
expect_false(small_table %>% has_columns(matches("z")))

# Expect FALSE when *any* of the given column names is absent
expect_false(small_table %>% has_columns(c(a, z)))

# Genuine evaluation errors are re-thrown and short-circuits
expect_error(small_table %>% has_columns(stop("Oh no!")), "Oh no!")
expect_error(small_table %>% has_columns(c(stop("Oh no!"))), "Oh no!")
Expand Down

0 comments on commit 21b3228

Please sign in to comment.