Skip to content

Commit 6a2bd14

Browse files
authored
check the input faceting names (#4922)
1 parent 10eba36 commit 6a2bd14

File tree

5 files changed

+44
-2
lines changed

5 files changed

+44
-2
lines changed

R/facet-.r

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,17 @@ check_layout <- function(x) {
486486
cli::cli_abort("Facet layout has a bad format. It must contain columns {.col PANEL}, {.col SCALE_X}, and {.col SCALE_Y}")
487487
}
488488

489+
check_facet_vars <- function(..., name) {
490+
vars_names <- c(...)
491+
reserved_names <- c("PANEL", "ROW", "COL", "SCALE_X", "SCALE_Y")
492+
problems <- intersect(vars_names, reserved_names)
493+
if (length(problems) != 0) {
494+
cli::cli_abort(c(
495+
"{.val {problems}} {?is/are} not {?an/} allowed name{?/s} for faceting variables",
496+
"i" = "Change the name of your data columns to not be {.or {.str {reserved_names}}}"
497+
), call = call2(name))
498+
}
499+
}
489500

490501
#' Get the maximal width/length of a list of grobs
491502
#'

R/facet-grid-.r

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,16 +196,18 @@ grid_as_facets_list <- function(rows, cols) {
196196
FacetGrid <- ggproto("FacetGrid", Facet,
197197
shrink = TRUE,
198198

199-
compute_layout = function(data, params) {
199+
compute_layout = function(self, data, params) {
200200
rows <- params$rows
201201
cols <- params$cols
202202

203+
check_facet_vars(names(rows), names(cols), name = snake_class(self))
204+
203205
dups <- intersect(names(rows), names(cols))
204206
if (length(dups) > 0) {
205207
cli::cli_abort(c(
206208
"Faceting variables can only appear in {.arg rows} or {.arg cols}, not both.\n",
207209
"i" = "Duplicated variables: {.val {dups}}"
208-
))
210+
), call = call2(snake_class(self)))
209211
}
210212

211213
base_rows <- combine_vars(data, params$plot_env, rows, drop = params$drop)

R/facet-wrap.r

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ FacetWrap <- ggproto("FacetWrap", Facet,
147147
return(layout_null())
148148
}
149149

150+
check_facet_vars(names(vars), name = snake_class(self))
151+
150152
base <- combine_vars(data, params$plot_env, vars, drop = params$drop)
151153

152154
id <- id(base, drop = TRUE)

tests/testthat/_snaps/facet-layout.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,18 @@
3939

4040
Free scales cannot be mixed with a fixed aspect ratio
4141

42+
# facet_wrap and facet_grid throws errors when using reserved words
43+
44+
"ROW" is not an allowed name for faceting variables
45+
i Change the name of your data columns to not be "PANEL", "ROW", "COL", "SCALE_X", or "SCALE_Y"
46+
47+
---
48+
49+
"ROW" and "PANEL" are not allowed names for faceting variables
50+
i Change the name of your data columns to not be "PANEL", "ROW", "COL", "SCALE_X", or "SCALE_Y"
51+
52+
---
53+
54+
"ROW" is not an allowed name for faceting variables
55+
i Change the name of your data columns to not be "PANEL", "ROW", "COL", "SCALE_X", or "SCALE_Y"
56+

tests/testthat/test-facet-layout.r

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,15 @@ test_that("facet_grid throws errors at bad layout specs", {
187187
theme(aspect.ratio = 1)
188188
expect_snapshot_error(ggplotGrob(p))
189189
})
190+
191+
test_that("facet_wrap and facet_grid throws errors when using reserved words", {
192+
mtcars2 <- mtcars
193+
mtcars2$PANEL <- mtcars2$cyl
194+
mtcars2$ROW <- mtcars2$gear
195+
196+
p <- ggplot(mtcars2) +
197+
geom_point(aes(mpg, disp))
198+
expect_snapshot_error(ggplotGrob(p + facet_grid(ROW ~ gear)))
199+
expect_snapshot_error(ggplotGrob(p + facet_grid(ROW ~ PANEL)))
200+
expect_snapshot_error(ggplotGrob(p + facet_wrap(~ROW)))
201+
})

0 commit comments

Comments
 (0)