Skip to content

Commit

Permalink
Merge pull request #601 from yjunechoe/glue-safely
Browse files Browse the repository at this point in the history
Fallback on failed glue in `label` and `brief`
  • Loading branch information
yjunechoe authored Feb 18, 2025
2 parents 0d8bc28 + e3eebfc commit 92899e9
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 10 deletions.
12 changes: 10 additions & 2 deletions R/steps_and_briefs.R
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,16 @@ create_validation_step <- function(
}

glue_chr <- function(x, glue_mask) {
if (is.na(x)) return(x)
as.character(glue::glue_data(glue_mask, x, .envir = NULL))
# TODO: also skip autobriefs (#273)
if (is.na(x) || !grepl("{", x, fixed = TRUE)) {
return(x)
}
# Try glue and fallback to original string if failed
out <- tryCatch(
expr = as.character(glue::glue_data(glue_mask, x, .envir = NULL)),
error = function(e) x
)
out
}

get_hash_version <- function() {
Expand Down
41 changes: 33 additions & 8 deletions tests/testthat/test-glue_label_brief.R
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,19 @@ test_that("label supports glue syntax for {.seg_val} {.seg_col} {.step} {.col}",
test_that("glue scope doesn't expose internal variables", {

# Ex: should not be able to access `columns` local variable in `col_vals_lt()`
expect_error(create_agent(small_table) %>% col_vals_lt(c, 8, label = "{columns}"))
expect_identical(
create_agent(small_table) %>%
col_vals_lt(c, 8, label = "{columns}") %>%
{.$validation_set$label},
"{columns}"
)
# Ex: should not be able to access `i` local variable in `create_validation_step()`
expect_error(create_agent(small_table) %>% col_vals_lt(c, 8, label = "{i}"))
expect_identical(
create_agent(small_table) %>%
col_vals_lt(c, 8, label = "{i}") %>%
{.$validation_set$label},
"{i}"
)

# Should be able to access global vars/fns
expect_equal(
Expand All @@ -83,23 +93,23 @@ test_that("glue scope doesn't expose internal variables", {

test_that("glue env searches from the caller env of the validation function", {

to_upper <- function(x) stop("Oh no!")
to_upper <- function(x) "global"

expect_error(
expect_identical(
create_agent(small_table) %>%
col_vals_lt(c, 8, label = "{to_upper(.col)}") %>%
{.$validation_set$label},
"Oh no!"
"global"
)

expect_equal(
expect_identical(
local({
to_upper <- function(x) toupper(x)
to_upper <- function(x) "local"
create_agent(small_table) %>%
col_vals_lt(c, 8, label = "{to_upper(.col)}") %>%
{.$validation_set$label}
}),
"C"
"local"
)

})
Expand Down Expand Up @@ -281,3 +291,18 @@ test_that("glue works for brief too", {
)

})

test_that("safe fallback when glue fails (#598)", {

expect_no_error({
agent <- create_agent(small_table) %>%
col_vals_regex(b, "^\\d{1,2}") %>%
interrogate()
})

expect_identical(
agent$validation_set$brief,
generate_autobriefs(agent, "b", NULL, "^\\d{1,2}", "col_vals_regex"),
)

})

0 comments on commit 92899e9

Please sign in to comment.