diff --git a/NEWS.md b/NEWS.md index b0706b15da..73c9d15683 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 (development version) +* Using two ordered factors as facetting variables in + `facet_grid(..., as.table = FALSE)` now throws a warning instead of an + error (@teunbrand, #5109). * Added `scale_linewidth_manual()` and `scale_linewidth_identity()` to support the `linewidth` aesthetic (@teunbrand, #5050). * Automatic breaks in `scale_*_binned()` should no longer be out-of-bounds, diff --git a/R/utilities.r b/R/utilities.r index 7d7fa21acc..6451d2e8eb 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -633,7 +633,8 @@ with_ordered_restart <- function(expr, .call) { restart <- TRUE if (is.ordered(x)) { x <- factor(as.character(x), levels = levels(x)) - } else { + } + if (is.ordered(y)) { y <- factor(as.character(y), levels = levels(y)) } } else if (is.character(x) || is.character(y)) { diff --git a/tests/testthat/test-utilities.r b/tests/testthat/test-utilities.r index fc4182e77e..b1f74127a0 100644 --- a/tests/testthat/test-utilities.r +++ b/tests/testthat/test-utilities.r @@ -127,3 +127,34 @@ test_that("cut_*() checks its input and output", { test_that("interleave() checks the vector lengths", { expect_snapshot_error(interleave(1:4, numeric())) }) + +test_that("vec_rbind0 can combined ordered factors", { + + withr::local_options(lifecycle_verbosity = "warning") + + # Ideally code below throws just 1 warning (the and one) + # However, it was technically challenging to reduce the numbers of warnings + # See #5139 for more details + + expect_warning( + expect_warning( + expect_warning( + { + test <- vec_rbind0( + data_frame0(a = factor(c("A", "B"), ordered = TRUE)), + data_frame0(a = factor(c("B", "C"), ordered = TRUE)) + ) + }, + " and ", class = "lifecycle_warning_deprecated" + ), + " and ", class = "lifecycle_warning_deprecated" + ), + " and ", class = "lifecycle_warning_deprecated" + ) + + # Should be not , hence the 'exact' + expect_s3_class(test$a, "factor", exact = TRUE) + # Test levels are combined sensibly + expect_equal(levels(test$a), c("A", "B", "C")) + +})