Skip to content

Clearer error messages for invalid aesthetics #3091

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Apr 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ core developer team.

* ggplot2 now works in Turkish locale (@yutannihilation, #3011).

* Clearer error messages for inappropriate aesthetics (@clairemcwhite, #3060).

* ggplot2 no longer attaches any external packages when using functions that
depend on packages that are suggested but not imported by ggplot2. The
affected functions include `geom_hex()`, `stat_binhex()`,
Expand Down
22 changes: 22 additions & 0 deletions R/layer.r
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,16 @@ Layer <- ggproto("Layer", NULL,
evaled <- lapply(aesthetics, eval_tidy, data = data)
evaled <- compact(evaled)

nondata_cols <- check_nondata_cols(evaled)
if (length(nondata_cols) > 0) {
msg <- paste0(
"Aesthetics must be valid data columns. Problematic aesthetic(s): ",
paste0(vapply(nondata_cols, function(x) {paste0(x, " = ", as_label(aesthetics[[x]]))}, character(1)), collapse = ", "),
". \nDid you mistype the name of a data column or forget to add stat()?"
)
stop(msg, call. = FALSE)
}

n <- nrow(data)
if (n == 0) {
# No data, so look at longest evaluated aesthetic
Expand Down Expand Up @@ -280,6 +290,18 @@ Layer <- ggproto("Layer", NULL,
env$stat <- stat

stat_data <- new_data_frame(lapply(new, eval_tidy, data, env))

# Check that all columns in aesthetic stats are valid data
nondata_stat_cols <- check_nondata_cols(stat_data)
if (length(nondata_stat_cols) > 0) {
msg <- paste0(
"Aesthetics must be valid computed stats. Problematic aesthetic(s): ",
paste0(vapply(nondata_stat_cols, function(x) {paste0(x, " = ", as_label(aesthetics[[x]]))}, character(1)), collapse = ", "),
". \nDid you map your stat in the wrong layer?"
)
stop(msg, call. = FALSE)
}

names(stat_data) <- names(new)

# Add any new scales, if needed
Expand Down
8 changes: 8 additions & 0 deletions R/utilities.r
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,14 @@ is.discrete <- function(x) {
is.factor(x) || is.character(x) || is.logical(x)
}

# This function checks that all columns of a dataframe `x` are data and
# returns the names of any columns that are not.
# We define "data" as atomic types or lists, not functions or otherwise
check_nondata_cols <- function(x) {
idx <- (vapply(x, function(x) rlang::is_vector(x), logical(1)))
names(x)[which(!idx)]
}

compact <- function(x) {
null <- vapply(x, is.null, logical(1))
x[!null]
Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/test-layer.r
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,24 @@ test_that("missing aesthetics trigger informative error", {
)
})

test_that("function aesthetics are wrapped with stat()", {
df <- data_frame(x = 1:10)
expect_error(
ggplot_build(ggplot(df, aes(colour = density, fill = density)) + geom_point()),
"Aesthetics must be valid data columns. Problematic aesthetic(s): colour = density, fill = density",
fixed = TRUE
)
})

test_that("computed stats are in appropriate layer", {
df <- data_frame(x = 1:10)
expect_error(
ggplot_build(ggplot(df, aes(colour = stat(density), fill = stat(density))) + geom_point()),
"Aesthetics must be valid computed stats. Problematic aesthetic(s): colour = stat(density), fill = stat(density)",
fixed = TRUE
)
})

test_that("if an aes is mapped to a function that returns NULL, it is removed", {
df <- data_frame(x = 1:10)
null <- function(...) NULL
Expand Down