Skip to content

Commit

Permalink
Merge pull request #504 from yjunechoe/info_columns-dummy-header
Browse files Browse the repository at this point in the history
Use richer dummy data for resolving columns of a lazy tbl informant
  • Loading branch information
yjunechoe committed Nov 6, 2023
2 parents d2ad537 + 1d491ba commit 2d8b166
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 18 deletions.
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## New features

* Complete `{tidyselect}` support for the `columns` argument of *all validation functions*, as well as in `has_columns()`. `columns` can now takes familiar column-selection expressions as one would use inside `dplyr::select()`. This also begins a process of deprecation:
* Complete `{tidyselect}` support for the `columns` argument of *all validation functions*, as well as in `has_columns()` and `info_columns`. `columns` can now takes familiar column-selection expressions as one would use inside `dplyr::select()`. This also begins a process of deprecation:
- `columns = vars(...)` will continue to work, but `c()` now supersedes `vars()`.
- If passing an *external vector* of column names, it should be wrapped in `all_of()`.

Expand Down
7 changes: 6 additions & 1 deletion R/create_informant.R
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,10 @@ create_informant <- function(
}
}

private <- list(
col_ptypes = tbl_info$col_ptypes
)

metadata_list <-
c(
list(
Expand All @@ -409,7 +413,8 @@ create_informant <- function(
`_type` = table_type
)
),
column_list
column_list,
list(`_private` = private)
)

# Create the metadata list object
Expand Down
2 changes: 2 additions & 0 deletions R/get_informant_report.R
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ get_informant_report <- function(
y <- informant$metadata_rev
} else {
y <- informant$metadata
# Hide private metadata from report
y[["_private"]] <- NULL
}

if ("info_label" %in% names(informant)) {
Expand Down
2 changes: 1 addition & 1 deletion R/incorporate.R
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ incorporate <- function(informant) {
extra_sections <-
base::setdiff(
names(informant$metadata),
c("info_label", "table", "columns")
c("info_label", "table", "columns", "_private")
)

metadata_extra <- informant$metadata[extra_sections]
Expand Down
2 changes: 1 addition & 1 deletion R/info_add.R
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ info_columns <- function(
metadata_columns <- metadata_list$columns

if (is.null(x$tbl)) {
tbl <- dplyr::as_tibble(metadata_columns %>% lapply(function(x) 1))
tbl <- metadata_list[["_private"]]$col_ptypes
} else {
tbl <- x$tbl
}
Expand Down
25 changes: 14 additions & 11 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -758,19 +758,18 @@ get_tbl_dbi_src_details <- function(tbl) {
get_r_column_names_types <- function(tbl) {

suppressWarnings(
column_names_types <-
column_header <-
tbl %>%
utils::head(1) %>%
dplyr::collect() %>%
vapply(
FUN.VALUE = character(1),
FUN = function(x) class(x)[1]
)
dplyr::collect()
)
column_names_types <-
vapply(column_header, function(x) class(x)[1], character(1))

list(
col_names = names(column_names_types),
r_col_types = unname(unlist(column_names_types))
r_col_types = unname(unlist(column_names_types)),
col_ptypes = utils::head(column_header, 0)
)
}

Expand Down Expand Up @@ -825,7 +824,8 @@ get_tbl_information_df <- function(tbl) {
db_tbl_name = NA_character_,
col_names = r_column_names_types$col_names,
r_col_types = r_column_names_types$r_col_types,
db_col_types = NA_character_
db_col_types = NA_character_,
col_ptypes = r_column_names_types$col_ptypes
)
}

Expand All @@ -849,7 +849,8 @@ get_tbl_information_spark <- function(tbl) {
db_tbl_name = NA_character_,
col_names = r_column_names_types$col_names,
r_col_types = r_column_names_types$r_col_types,
db_col_types = db_col_types
db_col_types = db_col_types,
col_ptypes = r_column_names_types$col_ptypes
)
}

Expand Down Expand Up @@ -1030,7 +1031,8 @@ get_tbl_information_dbi <- function(tbl) {
db_tbl_name = db_tbl_name,
col_names = r_column_names_types$col_names,
r_col_types = r_column_names_types$r_col_types,
db_col_types = db_col_types
db_col_types = db_col_types,
col_ptypes = r_column_names_types$col_ptypes
)
}

Expand Down Expand Up @@ -1074,7 +1076,8 @@ get_tbl_information_arrow <- function(tbl) {
db_tbl_name = NA_character_,
col_names = col_names,
r_col_types = r_col_types,
db_col_types = db_col_types
db_col_types = db_col_types,
col_ptypes = dplyr::collect(utils::head(tbl, 0))
)
}

Expand Down
5 changes: 4 additions & 1 deletion R/yaml_write.R
Original file line number Diff line number Diff line change
Expand Up @@ -1612,6 +1612,9 @@ as_informant_yaml_list <- function(informant) {
lst_locale <- list(locale = informant$locale)
}

# Hide private field
metadata <- informant$metadata[names(informant$metadata) != "_private"]

c(
type = "informant", # YAML type: `informant`
lst_read_fn, # table-prep formula (stored in key `tbl`)
Expand All @@ -1620,7 +1623,7 @@ as_informant_yaml_list <- function(informant) {
lst_lang, # informant language
lst_locale, # informant locale
lst_meta_snippets, # informant metadata snippet statements
informant$metadata # informant metadata entries
metadata # informant metadata entries
)
}

Expand Down
51 changes: 51 additions & 0 deletions tests/testthat/test-get_informant_report.R
Original file line number Diff line number Diff line change
Expand Up @@ -309,3 +309,54 @@ test_that("The correct title is rendered in the informant report", {
# )
# )
})

test_that("tidyselect integration in info_columns()", {
informant <- create_informant(small_table)
informant_lazy <- create_informant(~ small_table)

# Column headers stored in `$_private`
expect_s3_class(informant$metadata[["_private"]]$col_ptypes, "data.frame")
expect_s3_class(informant_lazy$metadata[["_private"]]$col_ptypes, "data.frame")

cols_with_info <- function(x) {
cols_info <- sapply(x$metadata$columns, `[[`, "info")
names(which(lengths(cols_info) > 0))
}
is_timepoint <- function(x) {
inherits(x, c("POSIXt", "POSIXct", "POSIXlt", "Date"))
}

# String-based and class-based matches both work
expect_identical({
informant %>%
info_columns(
columns = starts_with("date"),
info = "Time information"
) %>%
cols_with_info()
}, {
informant %>%
info_columns(
columns = where(is_timepoint),
info = "Time information"
) %>%
cols_with_info()
})
# String-based and class-based matches both work
expect_identical({
informant_lazy %>%
info_columns(
columns = starts_with("date"),
info = "Time information"
) %>%
cols_with_info()
}, {
informant_lazy %>%
info_columns(
columns = where(is_timepoint),
info = "Time information"
) %>%
cols_with_info()
})

})
2 changes: 2 additions & 0 deletions tests/testthat/test-has_columns.R
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
z <- rlang::missing_arg()

test_that("the `has_columns()` function works when used directly with data", {

# Expect TRUE when *all* of the given column names is present
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-incorporate_with_informant.R
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ test_that("Incorporating an informant yields the correct results", {
# `informant$metadata_rev`
expect_equal(
names(informant_inc$metadata),
c("table", "columns", "rows")
c("table", "columns", "_private", "rows")
)
expect_equal(
names(informant_inc$metadata_rev),
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-tidyselect_integration.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ tbl <- data.frame(x = 1:2, y = 1:2, nonunique = "A")
exist_col <- "y"
nonunique_col <- "nonunique"
nonexist_col <- "z"
z <- rlang::missing_arg()

test_that("Backwards compatibility with `vars()`", {

Expand Down
3 changes: 2 additions & 1 deletion tests/testthat/test-yaml_read_informant.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ test_that("Reading an informant from YAML is possible", {
section_name = "rows",
row_count = "There are {row_count} rows available."
)

yaml_write(informant, filename = "informant-test_table.yml")

informant_from_yaml <- yaml_read_informant(filename = "informant-test_table.yml")
Expand All @@ -60,6 +59,8 @@ test_that("Reading an informant from YAML is possible", {
# Expect that the informant (which never had `incorporate()`
# run on it) is equivalent to the informant object created
# via `yaml_read_informant()` (i.e., reading in the YAML file)
# - *Except* private fields which are note written
informant$metadata$`_private` <- NULL
expect_equivalent(informant, informant_from_yaml)

# Use `incorporate()` on the informant; this creates the list
Expand Down

0 comments on commit 2d8b166

Please sign in to comment.