Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bugfix has_columns() passing character vector of non-existing columns #540

Merged
merged 2 commits into from
Jun 15, 2024

Conversation

yjunechoe
Copy link
Collaborator

@yjunechoe yjunechoe commented Jun 15, 2024

I introduced a regression in #539 when I let hard-coded character vectors bypass the tidyselect-based column resolving mechanism (in resolve_columns()). This introduced a side-effect in has_columns() (which shares the same mechanism), making it pass character vectors through without checking for existence:

library(pointblank)

# Thinks that it "found" the column `y`
data.frame(x = 1) %>% has_columns("y")
#> [1] TRUE

This happened because a column selecting expression may successfully resolve to a character vector, but the columns in the vector themselves may not exist in the data. has_columns() was missing a check for the latter.

The PR ensures that has_columns() does an additional non-lazy check for existence of the resolved columns from the data - since active = ... has_columns() ... is called during interrogate, the tbl is resolved to an actual dataframe by then.

Now all variants align in producing the expected behavior when checking for a non-existent column:

devtools::load_all()
any(c(
  data.frame(x = 1) %>% has_columns(y),
  data.frame(x = 1) %>% has_columns("y"),
  data.frame(x = 1) %>% has_columns(vars(y)),
  data.frame(x = 1) %>% has_columns(vars("y"))
))
#> [1] FALSE

I've added more unit tests to cover this nuance specific to has_columns() (so this shouldn't accidentally regress again!)

Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yjunechoe yjunechoe merged commit 21b3228 into rstudio:main Jun 15, 2024
12 of 13 checks passed
@yjunechoe yjunechoe deleted the has_columns-character-bugfix branch June 15, 2024 13:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants