Skip to content

Commit e6cbcc1

Browse files
authored
Fix #3498 Restore data order after collide (#3680)
1 parent 351eb41 commit e6cbcc1

File tree

6 files changed

+88
-20
lines changed

6 files changed

+88
-20
lines changed

R/position-collide.r

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ collide <- function(data, width = NULL, name, strategy,
3838
# Reorder by x position, then on group. The default stacking order reverses
3939
# the group in order to match the legend order.
4040
if (reverse) {
41-
data <- data[order(data$xmin, data$group), ]
41+
ord <- order(data$xmin, data$group)
4242
} else {
43-
data <- data[order(data$xmin, -data$group), ]
43+
ord <- order(data$xmin, -data$group)
4444
}
45+
data <- data[ord, ]
4546

4647
# Check for overlap
4748
intervals <- as.numeric(t(unique(data[c("xmin", "xmax")])))
@@ -54,15 +55,15 @@ collide <- function(data, width = NULL, name, strategy,
5455
}
5556

5657
if (!is.null(data$ymax)) {
57-
dapply(data, "xmin", strategy, ..., width = width)
58+
data <- dapply(data, "xmin", strategy, ..., width = width)
5859
} else if (!is.null(data$y)) {
5960
data$ymax <- data$y
6061
data <- dapply(data, "xmin", strategy, ..., width = width)
6162
data$y <- data$ymax
62-
data
6363
} else {
6464
abort("Neither y nor ymax defined")
6565
}
66+
data[match(seq_along(ord), ord), ]
6667
}
6768

6869
# Alternate version of collide() used by position_dodge2()

tests/figs/deps.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
- vdiffr-svg-engine: 1.0
2-
- vdiffr: 0.3.0
2+
- vdiffr: 0.3.1
33
- freetypeharfbuzz: 0.2.5
Lines changed: 53 additions & 0 deletions
Loading

tests/figs/scales-breaks-and-labels/functional-limits.svg

Lines changed: 7 additions & 7 deletions
Loading

tests/testthat/test-position-stack.R

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
context("position_stack")
22

3-
test_that("data is sorted prior to stacking", {
3+
test_that("data keeps its order after stacking", {
44
df <- data_frame(
55
x = rep(c(1:10), 3),
66
var = rep(c("a", "b", "c"), 10),
@@ -9,7 +9,8 @@ test_that("data is sorted prior to stacking", {
99
p <- ggplot(df, aes(x = x, y = y, fill = var)) +
1010
geom_area(position = "stack")
1111
dat <- layer_data(p)
12-
expect_true(all(dat$group == 3:1))
12+
expect_true(all(dat$group == rep(1:3, each = 10)))
13+
expect_true(all(dat$x == df$x))
1314
})
1415

1516
test_that("negative and positive values are handled separately", {
@@ -21,8 +22,8 @@ test_that("negative and positive values are handled separately", {
2122
p <- ggplot(df, aes(x, y, fill = factor(g))) + geom_col()
2223
dat <- layer_data(p)
2324

24-
expect_equal(dat$ymin[dat$x == 1], c(0, -1, 1))
25-
expect_equal(dat$ymax[dat$x == 1], c(1, 0, 2))
25+
expect_equal(dat$ymin[dat$x == 1], c(1, -1, 0))
26+
expect_equal(dat$ymax[dat$x == 1], c(2, 0, 1))
2627

2728
expect_equal(dat$ymin[dat$x == 2], c(0, -3))
2829
expect_equal(dat$ymax[dat$x == 2], c(2, 0))
@@ -49,12 +50,25 @@ test_that("data with no extent is stacked correctly", {
4950
p0 <- base + geom_text(aes(label = y), position = position_stack(vjust = 0))
5051
p1 <- base + geom_text(aes(label = y), position = position_stack(vjust = 1))
5152

52-
expect_equal(layer_data(p0)$y, c(-75, -115))
53-
expect_equal(layer_data(p1)$y, c(0, -75))
53+
expect_equal(layer_data(p0)$y, c(-115, -75))
54+
expect_equal(layer_data(p1)$y, c(-75, 0))
5455
})
5556

5657
test_that("position_stack() can stack correctly when ymax is NA", {
5758
df <- data_frame(x = c(1, 1), y = c(1, 1))
5859
p <- ggplot(df, aes(x, y, ymax = NA_real_)) + geom_point(position = "stack")
5960
expect_equal(layer_data(p)$y, c(1, 2))
6061
})
62+
63+
# Visual tests ------------------------------------------------------------
64+
65+
test_that("Stacking produces the expected output", {
66+
data <- data_frame(
67+
x = rep(1:4, each = 2),
68+
category = rep(c("A","B"), 4),
69+
value = c(0, 0, 2, 1, 3, 6, -4, 3)
70+
)
71+
p <- ggplot(data, aes(x = x, y = value, fill = category)) +
72+
geom_area(stat = "identity")
73+
expect_doppelganger("Area stacking", p)
74+
})

tests/testthat/test-position_dodge.R

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ test_that("can control whether to preserve total or individual width", {
88
p_single <- ggplot(df, aes(x, fill = y)) +
99
geom_bar(position = position_dodge(preserve = "single"), width = 1)
1010

11-
expect_equal(layer_data(p_total)$x, c(1, 2.25, 1.75))
12-
expect_equal(layer_data(p_single)$x, c(0.75, 2.25, 1.75))
11+
expect_equal(layer_data(p_total)$x, c(1, 1.75, 2.25))
12+
expect_equal(layer_data(p_single)$x, c(0.75, 1.75, 2.25))
1313
})

0 commit comments

Comments
 (0)