diff --git a/NEWS.md b/NEWS.md index f3275ccf7c..bfeaffcbae 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # ggplot2 (development version) +* Prevented `facet_wrap(..., drop = FALSE)` from throwing spurious errors when + a character facetting variable contained `NA`s (@teunbrand, #5485). +* When facets coerce the faceting variables to factors, the 'ordered' class + is dropped (@teunbrand, #5666). * `geom_curve()` now appropriately removes missing data instead of throwing errors (@teunbrand, #5831). * `update_geom_defaults()` and `update_stat_defaults()` have a reset mechanism diff --git a/R/facet-grid-.R b/R/facet-grid-.R index 43897e4c9c..54c4b8f971 100644 --- a/R/facet-grid-.R +++ b/R/facet-grid-.R @@ -511,11 +511,11 @@ FacetGrid <- ggproto("FacetGrid", Facet, # Helpers ----------------------------------------------------------------- -ulevels <- function(x) { +ulevels <- function(x, na.last = TRUE) { if (is.factor(x)) { x <- addNA(x, TRUE) factor(levels(x), levels(x), exclude = NULL) } else { - sort(unique0(x)) + sort(unique0(x), na.last = na.last) } } diff --git a/R/stat-summary-2d.R b/R/stat-summary-2d.R index 30b61e6f58..60e5e49813 100644 --- a/R/stat-summary-2d.R +++ b/R/stat-summary-2d.R @@ -126,7 +126,7 @@ StatSummary2d <- ggproto("StatSummary2d", Stat, # Adaptation of tapply that returns a data frame instead of a matrix tapply_df <- function(x, index, fun, ..., drop = TRUE) { - labels <- lapply(index, ulevels) + labels <- lapply(index, ulevels, na.last = NA) # drop NA out <- expand.grid(labels, KEEP.OUT.ATTRS = FALSE, stringsAsFactors = FALSE) grps <- split(x, index) diff --git a/tests/testthat/test-facet-layout.R b/tests/testthat/test-facet-layout.R index 9ab6e80eeb..c22d1c36ca 100644 --- a/tests/testthat/test-facet-layout.R +++ b/tests/testthat/test-facet-layout.R @@ -100,7 +100,8 @@ test_that("grid: as.table reverses rows", { a2 <- data_frame( a = factor(1:3, levels = 1:4), - b = factor(1:3, levels = 4:1) + b = factor(1:3, levels = 4:1), + c = as.character(c(1:2, NA)) ) test_that("wrap: drop = FALSE preserves unused levels", { @@ -111,6 +112,11 @@ test_that("wrap: drop = FALSE preserves unused levels", { wrap_b <- panel_layout(facet_wrap(~b, drop = FALSE), list(a2)) expect_equal(nrow(wrap_b), 4) expect_equal(as.character(wrap_b$b), as.character(4:1)) + + # NA character should not be dropped or throw errors #5485 + wrap_c <- panel_layout(facet_wrap(~c, drop = FALSE), list(a2)) + expect_equal(nrow(wrap_c), 3) + expect_equal(wrap_c$c, a2$c) }) test_that("grid: drop = FALSE preserves unused levels", {