Skip to content

Commit 3d57b8f

Browse files
committed
Ensure LayerSf is stateless wrt legend type determination
1 parent 3522227 commit 3d57b8f

File tree

2 files changed

+26
-13
lines changed

2 files changed

+26
-13
lines changed

R/layer-sf.R

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ layer_sf <- function(geom = NULL, stat = NULL,
3232
}
3333

3434
LayerSf <- ggproto("LayerSf", Layer,
35+
legend_key_type = NULL,
36+
37+
# This field carry state throughout rendering but will always be
38+
# calculated before use
39+
computed_legend_key_type = NULL,
40+
3541
setup_layer = function(self, data, plot) {
3642
# process generic layer setup first
3743
data <- ggproto_parent(Layer, self)$setup_layer(data, plot)
@@ -50,24 +56,31 @@ LayerSf <- ggproto("LayerSf", Layer,
5056
# automatically determine the legend type
5157
if (is.null(self$legend_key_type)) {
5258
# first, set default value in case downstream tests fail
53-
self$geom_params$legend <- "polygon"
59+
self$computed_legend_key_type <- "polygon"
5460

5561
# now check if the type should not be polygon
5662
if (!is.null(self$computed_mapping$geometry) && quo_is_symbol(self$computed_mapping$geometry)) {
5763
geometry_column <- as_name(self$computed_mapping$geometry)
5864
if (inherits(data[[geometry_column]], "sfc")) {
5965
sf_type <- detect_sf_type(data[[geometry_column]])
6066
if (sf_type == "point") {
61-
self$geom_params$legend <- "point"
67+
self$computed_legend_key_type <- "point"
6268
} else if (sf_type == "line") {
63-
self$geom_params$legend <- "line"
69+
self$computed_legend_key_type <- "line"
6470
}
6571
}
6672
}
6773
} else {
68-
self$geom_params$legend <- self$legend_key_type
74+
self$computed_legend_key_type <- self$legend_key_type
6975
}
7076
data
77+
},
78+
compute_geom_1 = function(self, data) {
79+
data <- ggproto_parent(Layer, self)$compute_geom_1(data)
80+
81+
# Add legend type after computed_geom_params has been calculated
82+
self$computed_geom_params$legend <- self$computed_legend_key_type
83+
data
7184
}
7285
)
7386

tests/testthat/test-geom-sf.R

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,23 @@ test_that("geom_sf() determines the legend type automatically", {
3333

3434
# test the automatic choice
3535
expect_identical(fun_geom_sf(mp, TRUE)$plot$layers[[1]]$show.legend, TRUE)
36-
expect_identical(fun_geom_sf(mp, TRUE)$plot$layers[[1]]$geom_params$legend, "point")
36+
expect_identical(fun_geom_sf(mp, TRUE)$plot$layers[[1]]$computed_geom_params$legend, "point")
3737

3838
expect_identical(fun_geom_sf(mls, TRUE)$plot$layers[[1]]$show.legend, TRUE)
39-
expect_identical(fun_geom_sf(mls, TRUE)$plot$layers[[1]]$geom_params$legend, "line")
39+
expect_identical(fun_geom_sf(mls, TRUE)$plot$layers[[1]]$computed_geom_params$legend, "line")
4040

4141
expect_identical(fun_geom_sf(mpol, TRUE)$plot$layers[[1]]$show.legend, TRUE)
42-
expect_identical(fun_geom_sf(mpol, TRUE)$plot$layers[[1]]$geom_params$legend, "polygon")
42+
expect_identical(fun_geom_sf(mpol, TRUE)$plot$layers[[1]]$computed_geom_params$legend, "polygon")
4343

4444
# test that automatic choice can be overridden manually
4545
expect_identical(fun_geom_sf(mp, "point")$plot$layers[[1]]$show.legend, TRUE)
46-
expect_identical(fun_geom_sf(mp, "point")$plot$layers[[1]]$geom_params$legend, "point")
46+
expect_identical(fun_geom_sf(mp, "point")$plot$layers[[1]]$computed_geom_params$legend, "point")
4747

4848
expect_identical(fun_geom_sf(mls, "point")$plot$layers[[1]]$show.legend, TRUE)
49-
expect_identical(fun_geom_sf(mls, "point")$plot$layers[[1]]$geom_params$legend, "point")
49+
expect_identical(fun_geom_sf(mls, "point")$plot$layers[[1]]$computed_geom_params$legend, "point")
5050

5151
expect_identical(fun_geom_sf(mpol, "point")$plot$layers[[1]]$show.legend, TRUE)
52-
expect_identical(fun_geom_sf(mpol, "point")$plot$layers[[1]]$geom_params$legend, "point")
52+
expect_identical(fun_geom_sf(mpol, "point")$plot$layers[[1]]$computed_geom_params$legend, "point")
5353
})
5454

5555
test_that("geom_sf() determines the legend type from mapped geometry column", {
@@ -70,19 +70,19 @@ test_that("geom_sf() determines the legend type from mapped geometry column", {
7070
p <- ggplot_build(
7171
ggplot(d_sf) + geom_sf(aes(geometry = g_point, colour = "a"))
7272
)
73-
expect_identical(p$plot$layers[[1]]$geom_params$legend, "point")
73+
expect_identical(p$plot$layers[[1]]$computed_geom_params$legend, "point")
7474

7575
p <- ggplot_build(
7676
ggplot(d_sf) + geom_sf(aes(geometry = g_line, colour = "a"))
7777
)
78-
expect_identical(p$plot$layers[[1]]$geom_params$legend, "line")
78+
expect_identical(p$plot$layers[[1]]$computed_geom_params$legend, "line")
7979

8080
# If `geometry` is not a symbol, `LayerSf$setup_layer()` gives up guessing
8181
# the legend type, and falls back to "polygon"
8282
p <- ggplot_build(
8383
ggplot(d_sf) + geom_sf(aes(geometry = identity(g_point), colour = "a"))
8484
)
85-
expect_identical(p$plot$layers[[1]]$geom_params$legend, "polygon")
85+
expect_identical(p$plot$layers[[1]]$computed_geom_params$legend, "polygon")
8686
})
8787

8888
test_that("geom_sf() removes rows containing missing aes", {

0 commit comments

Comments
 (0)