From 61eac9779518fb0f6670dfb0e0846f41b53cc78b Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Mon, 4 Nov 2019 18:53:45 +0900 Subject: [PATCH 1/5] Move non-visual tests to top in test-geom-sf.R --- tests/testthat/test-geom-sf.R | 65 +++++++++++++++++------------------ 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/tests/testthat/test-geom-sf.R b/tests/testthat/test-geom-sf.R index 6de5f00dfb..88db417978 100644 --- a/tests/testthat/test-geom-sf.R +++ b/tests/testthat/test-geom-sf.R @@ -1,5 +1,37 @@ context("geom-sf") +test_that("geom_sf() removes rows containing missing aes", { + skip_if_not_installed("sf") + if (packageVersion("sf") < "0.5.3") skip("Need sf 0.5.3") + + grob_xy_length <- function(x) { + g <- layer_grob(x)[[1]] + c(length(g$x), length(g$y)) + } + + pts <- sf::st_sf( + geometry = sf::st_sfc(sf::st_point(0:1), sf::st_point(1:2)), + size = c(1, NA), + shape = c("a", NA), + colour = c("red", NA) + ) + + p <- ggplot(pts) + geom_sf() + expect_warning( + expect_identical(grob_xy_length(p + aes(size = size)), c(1L, 1L)), + "Removed 1 rows containing missing values" + ) + expect_warning( + expect_identical(grob_xy_length(p + aes(shape = shape)), c(1L, 1L)), + "Removed 1 rows containing missing values" + ) + # default colour scale maps a colour even to a NA, so identity scale is needed to see if NA is removed + expect_warning( + expect_identical(grob_xy_length(p + aes(colour = colour) + scale_colour_identity()), + c(1L, 1L)), + "Removed 1 rows containing missing values" + ) +}) # Visual tests ------------------------------------------------------------ @@ -64,36 +96,3 @@ test_that("geom_sf_text() and geom_sf_label() draws correctly", { ggplot() + geom_sf_label(data = nc_3857, aes(label = NAME)) ) }) - -test_that("geom_sf() removes rows containing missing aes", { - skip_if_not_installed("sf") - if (packageVersion("sf") < "0.5.3") skip("Need sf 0.5.3") - - grob_xy_length <- function(x) { - g <- layer_grob(x)[[1]] - c(length(g$x), length(g$y)) - } - - pts <- sf::st_sf( - geometry = sf::st_sfc(sf::st_point(0:1), sf::st_point(1:2)), - size = c(1, NA), - shape = c("a", NA), - colour = c("red", NA) - ) - - p <- ggplot(pts) + geom_sf() - expect_warning( - expect_identical(grob_xy_length(p + aes(size = size)), c(1L, 1L)), - "Removed 1 rows containing missing values" - ) - expect_warning( - expect_identical(grob_xy_length(p + aes(shape = shape)), c(1L, 1L)), - "Removed 1 rows containing missing values" - ) - # default colour scale maps a colour even to a NA, so identity scale is needed to see if NA is removed - expect_warning( - expect_identical(grob_xy_length(p + aes(colour = colour) + scale_colour_identity()), - c(1L, 1L)), - "Removed 1 rows containing missing values" - ) -}) From 78ff350f7d46d4dd288159abcf3c3a76a3dd0419 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Mon, 4 Nov 2019 19:10:49 +0900 Subject: [PATCH 2/5] Add a failing test --- tests/testthat/test-geom-sf.R | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/testthat/test-geom-sf.R b/tests/testthat/test-geom-sf.R index 88db417978..016e69a96c 100644 --- a/tests/testthat/test-geom-sf.R +++ b/tests/testthat/test-geom-sf.R @@ -33,6 +33,27 @@ test_that("geom_sf() removes rows containing missing aes", { ) }) +test_that("geom_sf() handles alpha properly", { + skip_if_not_installed("sf") + if (packageVersion("sf") < "0.5.3") skip("Need sf 0.5.3") + + sfc <- sf::st_sfc( + sf::st_point(0:1), + sf::st_linestring(rbind(0:1, 1:2)), + sf::st_polygon(list(rbind(0:1, 1:2, 2:1, 0:1))) + ) + red <- "#FF0000FF" + p <- ggplot(sfc) + geom_sf(colour = red, fill = red, alpha = 0.5) + g <- layer_grob(p)[[1]] + + # alpha affects the colour of points and lines + expect_equal(g[[1]]$gp$col, alpha(red, 0.5)) + expect_equal(g[[2]]$gp$col, alpha(red, 0.5)) + # alpha doesn't affect the colour of polygons, but the fill + expect_equal(g[[3]]$gp$col, alpha(red, 1.0)) + expect_equal(g[[3]]$gp$fill, alpha(red, 0.5)) +}) + # Visual tests ------------------------------------------------------------ test_that("geom_sf draws correctly", { From 2e2eff64a25aabb2f929b9c6e4de79897ea09497 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Mon, 4 Nov 2019 19:18:36 +0900 Subject: [PATCH 3/5] Alpha affects linestring geom_sf() --- R/geom-sf.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/geom-sf.R b/R/geom-sf.R index 8cb6da9c38..655a756855 100644 --- a/R/geom-sf.R +++ b/R/geom-sf.R @@ -167,7 +167,7 @@ sf_grob <- function(x, lineend = "butt", linejoin = "round", linemitre = 10, na. }) alpha <- x$alpha %||% defaults$alpha[type_ind] col <- x$colour %||% defaults$colour[type_ind] - col[is_point] <- alpha(col[is_point], alpha[is_point]) + col[is_point | is_line] <- alpha(col[is_point | is_line], alpha[is_point | is_line]) fill <- x$fill %||% defaults$fill[type_ind] fill <- alpha(fill, alpha) size <- x$size %||% defaults$size[type_ind] From 9c163e31d15f6248d78f5f2005d8c4adbaa857c6 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Mon, 4 Nov 2019 19:23:59 +0900 Subject: [PATCH 4/5] Add a NEWS bullet --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index e1452e8f75..9c57b6f00a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -89,6 +89,8 @@ * Increase the default `nbin` of `guide_colourbar()` to place the ticks more precisely (#3508, @yutannihilation). +* `geom_sf()` no more ignores alpha on linestring geometry (#3589, @yutannihilation). + # ggplot2 3.2.1 This is a patch release fixing a few regressions introduced in 3.2.0 as well as From 3bd84979284aa73663b282ff9f608f3c0b0ae2e1 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Wed, 6 Nov 2019 08:37:42 +0900 Subject: [PATCH 5/5] Improve a NEWS item --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 9c57b6f00a..750af431e5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -89,7 +89,7 @@ * Increase the default `nbin` of `guide_colourbar()` to place the ticks more precisely (#3508, @yutannihilation). -* `geom_sf()` no more ignores alpha on linestring geometry (#3589, @yutannihilation). +* `geom_sf()` now applies alpha to linestring geometries (#3589, @yutannihilation). # ggplot2 3.2.1