Skip to content

Commit 35e146b

Browse files
authored
Merge pull request #5139 from teunbrand/ordered_vec_rbind0
Try coerce ordered in `vec_rbind0()`
2 parents 020c9eb + ec299fc commit 35e146b

File tree

3 files changed

+36
-1
lines changed

3 files changed

+36
-1
lines changed

NEWS.md

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# ggplot2 (development version)
22

3+
* Using two ordered factors as facetting variables in
4+
`facet_grid(..., as.table = FALSE)` now throws a warning instead of an
5+
error (@teunbrand, #5109).
36
* Added `scale_linewidth_manual()` and `scale_linewidth_identity()` to support
47
the `linewidth` aesthetic (@teunbrand, #5050).
58
* Automatic breaks in `scale_*_binned()` should no longer be out-of-bounds,

R/utilities.r

+2-1
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,8 @@ with_ordered_restart <- function(expr, .call) {
633633
restart <- TRUE
634634
if (is.ordered(x)) {
635635
x <- factor(as.character(x), levels = levels(x))
636-
} else {
636+
}
637+
if (is.ordered(y)) {
637638
y <- factor(as.character(y), levels = levels(y))
638639
}
639640
} else if (is.character(x) || is.character(y)) {

tests/testthat/test-utilities.r

+31
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,34 @@ test_that("cut_*() checks its input and output", {
127127
test_that("interleave() checks the vector lengths", {
128128
expect_snapshot_error(interleave(1:4, numeric()))
129129
})
130+
131+
test_that("vec_rbind0 can combined ordered factors", {
132+
133+
withr::local_options(lifecycle_verbosity = "warning")
134+
135+
# Ideally code below throws just 1 warning (the <ordered> and <ordered> one)
136+
# However, it was technically challenging to reduce the numbers of warnings
137+
# See #5139 for more details
138+
139+
expect_warning(
140+
expect_warning(
141+
expect_warning(
142+
{
143+
test <- vec_rbind0(
144+
data_frame0(a = factor(c("A", "B"), ordered = TRUE)),
145+
data_frame0(a = factor(c("B", "C"), ordered = TRUE))
146+
)
147+
},
148+
"<ordered> and <ordered>", class = "lifecycle_warning_deprecated"
149+
),
150+
"<ordered> and <factor>", class = "lifecycle_warning_deprecated"
151+
),
152+
"<ordered> and <factor>", class = "lifecycle_warning_deprecated"
153+
)
154+
155+
# Should be <factor> not <ordered/factor>, hence the 'exact'
156+
expect_s3_class(test$a, "factor", exact = TRUE)
157+
# Test levels are combined sensibly
158+
expect_equal(levels(test$a), c("A", "B", "C"))
159+
160+
})

0 commit comments

Comments
 (0)