From 6c7f08239fea5ccda1511251702d4f322aeee194 Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Sat, 25 Jul 2026 11:17:21 +0530 Subject: [PATCH 1/6] Split facet error messages for missing vars vs bad notation (#168). Detect facet_wrap(. ~ var) when vars exist and suggest ~var or "var" notation. Keep missing-variable errors separate without formula advice. Expand tests per tdhock review on PR #285. --- R/facet-layout.r | 30 ++++++- .../test-renderer-facet-error-messages.R | 87 +++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/test-renderer-facet-error-messages.R diff --git a/R/facet-layout.r b/R/facet-layout.r index ef312b4a1..13f85b9e1 100644 --- a/R/facet-layout.r +++ b/R/facet-layout.r @@ -48,6 +48,20 @@ layout_wrap <- function(data, vars = NULL, nrow = NULL, ncol = NULL, vars <- as.quoted(vars) if (length(vars) == 0) return(layout_null()) + # Notation error only when real vars exist; otherwise layout_base reports missing columns. + if ("." %in% names(vars)) { + real_vars <- names(vars)[names(vars) != "."] + all_data_vars <- unique(unlist(lapply(data, names))) + missing_vars <- setdiff(real_vars, all_data_vars) + if (length(missing_vars) == 0) { + stop(sprintf( + "Invalid facet_wrap formula (. ~ var). Use facet_wrap(~%s) or facet_wrap(\"%s\")", + paste(real_vars, collapse = " + "), + paste(real_vars, collapse = "\", \"") + ), call. = FALSE) + } + } + base <- plyr::unrowname(layout_base(data, vars, drop = drop)) id <- plyr::id(base, drop = TRUE) @@ -93,8 +107,22 @@ layout_base <- function(data, vars = NULL, drop = TRUE) { # Form the base data frame which contains all combinations of facetting # variables that appear in the data - has_all <- unlist(plyr::llply(values, length)) == length(vars) + has_all <- unlist(lapply(values, length)) == length(vars) if (!any(has_all)) { + all_data_vars <- unique(unlist(lapply(data, names))) + missing_vars <- setdiff(names(vars)[names(vars) != "."], all_data_vars) + if (length(missing_vars) > 0) { + stop(sprintf( + "Facet variable%s not found in data: %s\nAvailable columns: %s", + if (length(missing_vars) > 1) "s" else "", + paste(missing_vars, collapse = ", "), + paste(all_data_vars, collapse = ", ") + )) + } + # Reached when all facet variables exist somewhere in the data, but no + # single layer has all of them together. For example, facet_grid(X ~ Y) + # where layer 1 only has column X and layer 2 only has column Y — neither + # layer alone satisfies all facetting variables, so has_all is FALSE for every layer. stop("At least one layer must contain all variables used for facetting") } diff --git a/tests/testthat/test-renderer-facet-error-messages.R b/tests/testthat/test-renderer-facet-error-messages.R new file mode 100644 index 000000000..783fac592 --- /dev/null +++ b/tests/testthat/test-renderer-facet-error-messages.R @@ -0,0 +1,87 @@ +acontext("Facet error messages") +missing_var_error <- paste( + "Facet variable not found in data: NonExistentColumn", + "Available columns: Sepal.Length, Sepal.Width, Petal.Length, Petal.Width, Species", + sep = "\n" +) +missing_var_string_error <- paste( + "Facet variable not found in data: MissingVar", + "Available columns: Sepal.Length, Sepal.Width, Petal.Length, Petal.Width, Species", + sep = "\n" +) +bad_notation_error <- "Invalid facet_wrap formula (. ~ var). Use facet_wrap(~Species) or facet_wrap(\"Species\")" +test_that("facet_wrap string missing variable gives clear error", { + viz <- list( + scatter = ggplot() + + facet_wrap("MissingVar") + + geom_point(aes(Sepal.Length, Petal.Length), data = iris) + ) + expect_error( + animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), + missing_var_string_error, + fixed = TRUE + ) +}) +test_that("facet_wrap formula with missing variable gives clear error", { + viz <- list( + scatter = ggplot() + + facet_wrap(. ~ NonExistentColumn) + + geom_point(aes(Sepal.Length, Petal.Length), data = iris) + ) + expect_error( + animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), + missing_var_error, + fixed = TRUE + ) +}) +test_that("facet_wrap bad notation when variable exists gives clear error", { + viz <- list( + scatter = ggplot() + + facet_wrap(. ~ Species) + + geom_point(aes(Sepal.Length, Petal.Length), data = iris) + ) + expect_error( + animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), + bad_notation_error, + fixed = TRUE + ) +}) +test_that("facet_grid missing variable gives clear error", { + viz <- list( + scatter = ggplot() + + facet_grid(. ~ MissingVar) + + geom_point(aes(Sepal.Length, Petal.Length), data = iris) + ) + expect_error( + animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), + missing_var_string_error, + fixed = TRUE + ) +}) +test_that("facet_wrap tilde notation works", { + viz <- list( + scatter = ggplot() + + facet_wrap(~Species) + + geom_point(aes(Sepal.Length, Petal.Length), data = iris) + ) + info <- animint2dir(viz, out.dir = tempfile(), open.browser = FALSE) + expect_true(file.exists(file.path(info$out.dir, "index.html"))) +}) +test_that("facet_wrap string notation works", { + viz <- list( + scatter = ggplot() + + facet_wrap("Species") + + geom_point(aes(Sepal.Length, Petal.Length), data = iris) + ) + info <- animint2dir(viz, out.dir = tempfile(), open.browser = FALSE) + expect_true(file.exists(file.path(info$out.dir, "index.html"))) +}) +test_that("facet_grid formula notation works", { + viz <- list( + scatter = ggplot() + + facet_grid(. ~ Species) + + geom_point(aes(Sepal.Length, Petal.Length), data = iris) + ) + info <- animint2dir(viz, out.dir = tempfile(), open.browser = FALSE) + expect_true(file.exists(file.path(info$out.dir, "index.html"))) +}) From a4307cdffc50d34ffa4fd09b6a142a2869f9e17e Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Thu, 30 Jul 2026 11:39:22 +0530 Subject: [PATCH 2/6] test: align facet error tests with animint2dir conventions (#168) Use expect_error blocks with animint2dir(viz) and drop explicit out.dir in success tests per tdhock review on PR #285. --- .../test-renderer-facet-error-messages.R | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/tests/testthat/test-renderer-facet-error-messages.R b/tests/testthat/test-renderer-facet-error-messages.R index 783fac592..09805f24d 100644 --- a/tests/testthat/test-renderer-facet-error-messages.R +++ b/tests/testthat/test-renderer-facet-error-messages.R @@ -16,11 +16,9 @@ test_that("facet_wrap string missing variable gives clear error", { facet_wrap("MissingVar") + geom_point(aes(Sepal.Length, Petal.Length), data = iris) ) - expect_error( - animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), - missing_var_string_error, - fixed = TRUE - ) + expect_error({ + animint2dir(viz) + }, missing_var_string_error, fixed = TRUE) }) test_that("facet_wrap formula with missing variable gives clear error", { viz <- list( @@ -28,11 +26,9 @@ test_that("facet_wrap formula with missing variable gives clear error", { facet_wrap(. ~ NonExistentColumn) + geom_point(aes(Sepal.Length, Petal.Length), data = iris) ) - expect_error( - animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), - missing_var_error, - fixed = TRUE - ) + expect_error({ + animint2dir(viz) + }, missing_var_error, fixed = TRUE) }) test_that("facet_wrap bad notation when variable exists gives clear error", { viz <- list( @@ -40,11 +36,9 @@ test_that("facet_wrap bad notation when variable exists gives clear error", { facet_wrap(. ~ Species) + geom_point(aes(Sepal.Length, Petal.Length), data = iris) ) - expect_error( - animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), - bad_notation_error, - fixed = TRUE - ) + expect_error({ + animint2dir(viz) + }, bad_notation_error, fixed = TRUE) }) test_that("facet_grid missing variable gives clear error", { viz <- list( @@ -52,11 +46,9 @@ test_that("facet_grid missing variable gives clear error", { facet_grid(. ~ MissingVar) + geom_point(aes(Sepal.Length, Petal.Length), data = iris) ) - expect_error( - animint2dir(viz, out.dir = tempfile(), open.browser = FALSE), - missing_var_string_error, - fixed = TRUE - ) + expect_error({ + animint2dir(viz) + }, missing_var_string_error, fixed = TRUE) }) test_that("facet_wrap tilde notation works", { viz <- list( @@ -64,7 +56,7 @@ test_that("facet_wrap tilde notation works", { facet_wrap(~Species) + geom_point(aes(Sepal.Length, Petal.Length), data = iris) ) - info <- animint2dir(viz, out.dir = tempfile(), open.browser = FALSE) + info <- animint2dir(viz, open.browser = FALSE) expect_true(file.exists(file.path(info$out.dir, "index.html"))) }) test_that("facet_wrap string notation works", { @@ -73,7 +65,7 @@ test_that("facet_wrap string notation works", { facet_wrap("Species") + geom_point(aes(Sepal.Length, Petal.Length), data = iris) ) - info <- animint2dir(viz, out.dir = tempfile(), open.browser = FALSE) + info <- animint2dir(viz, open.browser = FALSE) expect_true(file.exists(file.path(info$out.dir, "index.html"))) }) test_that("facet_grid formula notation works", { @@ -82,6 +74,6 @@ test_that("facet_grid formula notation works", { facet_grid(. ~ Species) + geom_point(aes(Sepal.Length, Petal.Length), data = iris) ) - info <- animint2dir(viz, out.dir = tempfile(), open.browser = FALSE) + info <- animint2dir(viz, open.browser = FALSE) expect_true(file.exists(file.path(info$out.dir, "index.html"))) }) From dc26c99978781fddbdeddf55413f81d5bfbb2064 Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Sun, 23 Aug 2026 21:30:01 +0530 Subject: [PATCH 3/6] Wait after plot click before reading driver.js popover. R_coverage was failing on clicking top plot keeps driver open because click_center returned before the popover DOM settled. --- tests/testthat/test-renderer3-knit-print.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-renderer3-knit-print.R b/tests/testthat/test-renderer3-knit-print.R index e732b42c8..7b3aca2ad 100644 --- a/tests/testthat/test-renderer3-knit-print.R +++ b/tests/testthat/test-renderer3-knit-print.R @@ -181,6 +181,7 @@ click_center <- function(id){ ## https://github.com/rstudio/chromote/issues/32 do.call(remDr$Input$dispatchMouseEvent, L) } + Sys.sleep(1) } djs.init.list <- driverjs_get(html) From ec81fd507d9e9a4e31760f0ff6d1edf5c3426876 Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Wed, 26 Aug 2026 17:14:47 +0530 Subject: [PATCH 4/6] Retry transient GitHub Pages clone failures Handle temporary repository propagation delays without masking other clone errors. --- tests/testthat/test-compiler-ghpages.R | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-compiler-ghpages.R b/tests/testthat/test-compiler-ghpages.R index 70ef74125..4af477d8b 100644 --- a/tests/testthat/test-compiler-ghpages.R +++ b/tests/testthat/test-compiler-ghpages.R @@ -51,6 +51,28 @@ reset_test_repo <- function(){ gh::gh("POST /orgs/animint-test/repos", name = test_repo_name) Sys.sleep(3) } +clone_test_repo <- function(github_url, new_clone){ + max_attempts <- 5 + wait_seconds <- 3 + last_error <- NULL + for (attempt in seq_len(max_attempts)) { + result <- tryCatch({ + clone <- gert::git_clone(github_url, new_clone) + last_error <- NULL + clone + }, error = function(err){ + last_error <<- err + NULL + }) + if (is.null(last_error)) return(result) + if (!grepl("status code: 404", conditionMessage(last_error), fixed = TRUE)) { + stop(last_error) + } + unlink(new_clone, recursive = TRUE, force = TRUE) + if (attempt < max_attempts) Sys.sleep(wait_seconds) + } + stop(last_error) +} test_that("animint2pages(chromote_sleep_seconds=3) creates Capture.PNG", { reset_test_repo() ## first run of animint2pages creates new data viz. @@ -96,7 +118,7 @@ test_that("animint2pages(chromote_sleep_seconds=NULL) does not create Capture.PN ## clone and add Capture.PNG new_clone <- tempfile() github_url <- paste0("https://github.com/", result_list$owner_repo, ".git") - gert::git_clone(github_url, new_clone) + clone_test_repo(github_url, new_clone) branch_name <- gert::git_branch(new_clone) expect_identical(branch_name, "gh-pages", info=paste(branch_name, "should be gh-pages")) cat("FOO", file=file.path(new_clone, "Capture.PNG")) From a9a017f943a3746bec7429dc0eca0960dc3edfe1 Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Fri, 4 Sep 2026 22:39:46 +0530 Subject: [PATCH 5/6] Added NEWS and bump version for clearer facet errors (#285). --- DESCRIPTION | 2 +- NEWS.md | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index b8c2df633..3bc1148bf 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: animint2 Title: Animated Interactive Grammar of Graphics -Version: 2026.8.23 +Version: 2026.9.4 URL: https://animint.github.io/animint2 BugReports: https://github.com/animint/animint2/issues Authors@R: c( diff --git a/NEWS.md b/NEWS.md index b2d504529..67d248c9f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +# Changes in version 2026.9.4 (PR#285) + +- Clearer facet error messages when a facet variable is missing from the data, or when `facet_wrap(. ~ var)` uses invalid notation (issue #168). + # Changes in development (PR#339) - Multi-line facet strip labels: `facet_grid()` and `facet_wrap()` now render each facet variable on its own line, matching ggplot2 (issue #262). From e63d3229d68041b3c17be416f8b2e08d28291054 Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Mon, 28 Sep 2026 15:16:45 +0530 Subject: [PATCH 6/6] Fix CRAN NEWS note for facet error message release (#285). Remove unversioned development heading and bump to 2026.9.28. --- DESCRIPTION | 2 +- NEWS.md | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 5ca8f82a3..2203c1d72 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: animint2 Title: Animated Interactive Grammar of Graphics -Version: 2026.9.5 +Version: 2026.9.28 URL: https://animint.github.io/animint2 BugReports: https://github.com/animint/animint2/issues Authors@R: c( diff --git a/NEWS.md b/NEWS.md index 5fd0f52e0..744745d2c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,9 +1,7 @@ -# Changes in version 2026.9.4 (PR#285) +# Changes in version 2026.9.28 (PR#285) - Clearer facet error messages when a facet variable is missing from the data, or when `facet_wrap(. ~ var)` uses invalid notation (issue #168). -# Changes in development (PR#339) - # Changes in version 2026.9.5 - geom_text() renders with .text(), instead of setMultilineText(), fixing interactive updates.