From 34c01ccd3b22fadd9c662a5ef4b386469a3dec59 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Tue, 28 Aug 2018 12:14:43 -0400 Subject: [PATCH 01/18] use parse_safe() in geom_text() The built in `parse()` function silently drops items: length(parse(text = c("alpha", "", "gamma"))) We expect the length to be 3, but instead it is 2. So, we add a new function `parse_safe()` that keeps all items. This addresses issue #2864 --- R/geom-label.R | 2 +- R/geom-text.r | 3 +-- R/utilities.r | 11 +++++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/R/geom-label.R b/R/geom-label.R index 921f4784ac..3a1d3250b5 100644 --- a/R/geom-label.R +++ b/R/geom-label.R @@ -63,7 +63,7 @@ GeomLabel <- ggproto("GeomLabel", Geom, label.size = 0.25) { lab <- data$label if (parse) { - lab <- parse(text = as.character(lab)) + lab <- parse_safe(text = as.character(lab)) } data <- coord$transform(data, panel_params) diff --git a/R/geom-text.r b/R/geom-text.r index d86fee04a6..88be5c721d 100644 --- a/R/geom-text.r +++ b/R/geom-text.r @@ -159,7 +159,6 @@ geom_text <- function(mapping = NULL, data = NULL, ) } - #' @rdname ggplot2-ggproto #' @format NULL #' @usage NULL @@ -176,7 +175,7 @@ GeomText <- ggproto("GeomText", Geom, na.rm = FALSE, check_overlap = FALSE) { lab <- data$label if (parse) { - lab <- parse(text = as.character(lab)) + lab <- parse_safe(text = as.character(lab)) } data <- coord$transform(data, panel_params) diff --git a/R/utilities.r b/R/utilities.r index 06651d7251..4dc2950e5a 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -430,3 +430,14 @@ is_column_vec <- function(x) { dims <- dim(x) length(dims) == 2L && dims[[2]] == 1L } + +# Parse a vector of expressions without silently dropping any items. +# (see #2864) +parse_safe <- function(text) { + text <- as.character(text) + ix <- which(sapply(text, function(x) { + length(parse(text = x)) == 0 + })) + text[ix] <- NA + parse(text = text) +} From fe7216ad3df09f9074d3301b75078138084881e1 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Thu, 30 Aug 2018 08:38:21 -0400 Subject: [PATCH 02/18] update parse_safe() Use Hadley's code to parse the expressions once instead of twice. Use `parse(..., keep.source = FALSE)`, as Claus recommended. Add an example to demonstrate why `parse_safe()` is needed. --- R/utilities.r | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/R/utilities.r b/R/utilities.r index 4dc2950e5a..5e5ab6f9c0 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -431,13 +431,20 @@ is_column_vec <- function(x) { length(dims) == 2L && dims[[2]] == 1L } -# Parse a vector of expressions without silently dropping any items. -# (see #2864) +#' Parse a vector of expressions without silently dropping any items. +#' +#' For more discussion, see #2864 +#' @keywords internal +#' @examples +#' length(parse(text = c("alpha", "", "gamma"))) +#' #> 2 +#' length(parse_safe(text = c("alpha", "", "gamma"))) +#' #> 3 parse_safe <- function(text) { - text <- as.character(text) - ix <- which(sapply(text, function(x) { - length(parse(text = x)) == 0 - })) - text[ix] <- NA - parse(text = text) + out <- vector("expression", length(text)) + for (i in seq_along(text)) { + expr <- parse(text = text[[i]]) + out[[i]] <- if (length(expr) == 0) NA else expr[[1]] + } + out } From aa9bfc365aa604eea4c25d3c37f21a2e82891831 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Thu, 30 Aug 2018 08:40:11 -0400 Subject: [PATCH 03/18] add tests for parse_safe() --- tests/testthat/test-parse.r | 52 +++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/testthat/test-parse.r diff --git a/tests/testthat/test-parse.r b/tests/testthat/test-parse.r new file mode 100644 index 0000000000..67670e91df --- /dev/null +++ b/tests/testthat/test-parse.r @@ -0,0 +1,52 @@ +context("Parsing expressions") + +expect_equal( + parse_safe(c("", " ", " \n \n ")), + expression(NA, NA, NA) +) + +expect_equal( + parse_safe(c("A", "B", "C")), + expression(A, B, C) +) + +expect_equal( + parse_safe(c("alpha", "beta", "gamma")), + expression(alpha, beta, gamma) +) + +expect_equal( + parse_safe(c("alpha", "", "gamma", " ")), + expression(alpha, NA, gamma, NA) +) + +expect_equal( + parse_safe(c("alpha ~ beta", " ", "integral(f(x) * dx, a, b)")), + expression(alpha ~ beta, NA, integral(f(x) * dx, a, b)) +) + +expect_equal( + parse_safe(c("alpha \n beta", " ", "integral(f(x) * dx, a, b)")), + expression(alpha, NA, integral(f(x) * dx, a, b)) +) + +expect_equal( + parse_safe(c(NA, "a", NA, "alpha")), + expression(NA, a, NA, alpha) +) + +expect_equal( + parse_safe(factor(c("alpha", "beta", ""))), + expression(2, 3, 1) +) + +expect_equal( + parse_safe(factor(c(NA, "a", NA, "alpha"))), + expression(NA, 1, NA, 2) +) + +expect_equal( + parse_safe(c(NA, 1, 2, "a \n b")), + expression(NA, 1, 2, a) +) + From 5dd869e5992f0e61a0dc6c449e9905add5ad24dd Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Thu, 30 Aug 2018 08:55:36 -0400 Subject: [PATCH 04/18] add note about parse(text = NULL) --- R/utilities.r | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/utilities.r b/R/utilities.r index 5e5ab6f9c0..d8d214768b 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -433,7 +433,10 @@ is_column_vec <- function(x) { #' Parse a vector of expressions without silently dropping any items. #' +#' Note that `parse(text = NULL)` does not work and should be avoided. +#' #' For more discussion, see #2864 +#' #' @keywords internal #' @examples #' length(parse(text = c("alpha", "", "gamma"))) From 252e7b2a4fcd7c72602bb7f5e71e5ef6731f52b2 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Thu, 30 Aug 2018 13:40:30 -0400 Subject: [PATCH 05/18] use parse_safe() in sf.R This patch fixes an example that triggers an error: library(ggplot2) nc <- sf::st_read(system.file("shape/nc.shp", package = "sf"), quiet = TRUE) ggplot(nc) + geom_sf(aes(fill = AREA)) + scale_y_continuous( breaks = c(34, 35, 36), labels = c("34*degree*N", "", "36*degree*N") ) #> Error in parse(text = x)[[1]]: subscript out of bounds See #2867 for more details. --- R/sf.R | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/R/sf.R b/R/sf.R index 3d9ff0fae9..b2e76abf0b 100644 --- a/R/sf.R +++ b/R/sf.R @@ -476,10 +476,6 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, if (!is.null(graticule$plot12)) graticule$degree_label[!graticule$plot12] <- NA - # parse labels into expressions if required - if (any(grepl("degree", graticule$degree_label))) - graticule$degree_label <- lapply(graticule$degree_label, function(x) parse(text = x)[[1]]) - graticule }, @@ -553,11 +549,18 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, graticule <- panel_params$graticule east <- graticule[graticule$type == "E" & !is.na(graticule$degree_label), ] + labs <- east$degree_label + + # parse labels into expressions if required + if (any(grepl("degree", labs))) { + labs <- parse_safe(as.character(labs)) + } + list( top = nullGrob(), bottom = guide_axis( east$x_start, - east$degree_label, + labs, position = "bottom", theme = theme ) @@ -568,10 +571,17 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, graticule <- panel_params$graticule north <- graticule[graticule$type == "N" & !is.na(graticule$degree_label), ] + labs <- north$degree_label + + # parse labels into expressions if required + if (any(grepl("degree", labs))) { + labs <- parse_safe(as.character(labs)) + } + list( left = guide_axis( north$y_start, - north$degree_label, + labs, position = "left", theme = theme ), From 4b6b02f4ea2b34b86003555e6e2587814cb236ce Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Sun, 2 Sep 2018 22:03:46 -0400 Subject: [PATCH 06/18] move parse_safe() tests to test-utilities.r --- tests/testthat/test-parse.r | 52 --------------------------------- tests/testthat/test-utilities.r | 49 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 52 deletions(-) delete mode 100644 tests/testthat/test-parse.r diff --git a/tests/testthat/test-parse.r b/tests/testthat/test-parse.r deleted file mode 100644 index 67670e91df..0000000000 --- a/tests/testthat/test-parse.r +++ /dev/null @@ -1,52 +0,0 @@ -context("Parsing expressions") - -expect_equal( - parse_safe(c("", " ", " \n \n ")), - expression(NA, NA, NA) -) - -expect_equal( - parse_safe(c("A", "B", "C")), - expression(A, B, C) -) - -expect_equal( - parse_safe(c("alpha", "beta", "gamma")), - expression(alpha, beta, gamma) -) - -expect_equal( - parse_safe(c("alpha", "", "gamma", " ")), - expression(alpha, NA, gamma, NA) -) - -expect_equal( - parse_safe(c("alpha ~ beta", " ", "integral(f(x) * dx, a, b)")), - expression(alpha ~ beta, NA, integral(f(x) * dx, a, b)) -) - -expect_equal( - parse_safe(c("alpha \n beta", " ", "integral(f(x) * dx, a, b)")), - expression(alpha, NA, integral(f(x) * dx, a, b)) -) - -expect_equal( - parse_safe(c(NA, "a", NA, "alpha")), - expression(NA, a, NA, alpha) -) - -expect_equal( - parse_safe(factor(c("alpha", "beta", ""))), - expression(2, 3, 1) -) - -expect_equal( - parse_safe(factor(c(NA, "a", NA, "alpha"))), - expression(NA, 1, NA, 2) -) - -expect_equal( - parse_safe(c(NA, 1, 2, "a \n b")), - expression(NA, 1, 2, a) -) - diff --git a/tests/testthat/test-utilities.r b/tests/testthat/test-utilities.r index aa42cf49f1..a98c6052b1 100644 --- a/tests/testthat/test-utilities.r +++ b/tests/testthat/test-utilities.r @@ -43,3 +43,52 @@ test_that("find_args behaves correctly", { # Defaults are overwritten expect_true(test_fun(arg2 = TRUE)$arg2) }) + +test_that("parse_safe works with simple expressions", { + expect_equal( + parse_safe(c("", " ", " ")), + expression(NA, NA, NA) + ) + + expect_equal( + parse_safe(c("A", "B", "C")), + expression(A, B, C) + ) + + expect_equal( + parse_safe(c("alpha", "", "gamma", " ")), + expression(alpha, NA, gamma, NA) + ) + + expect_equal( + parse_safe(c(NA, "a", NA, "alpha")), + expression(NA, a, NA, alpha) + ) + + expect_equal( + parse_safe(factor(c("alpha", "beta", ""))), + expression(2, 3, 1) + ) +}) + +test_that("parse_safe works with multi expressions", { + expect_equal( + parse_safe(c(" \n", "\n ", " \n \n ")), + expression(NA, NA, NA) + ) + + expect_equal( + parse_safe(c("alpha ~ beta", "beta \n gamma", "")), + expression(alpha ~ beta, beta, NA) + ) + + expect_equal( + parse_safe(c("alpha ~ beta", " ", "integral(f(x) * dx, a, b)")), + expression(alpha ~ beta, NA, integral(f(x) * dx, a, b)) + ) + + expect_equal( + parse_safe(c(NA, 1, 2, "a \n b")), + expression(NA, 1, 2, a) + ) +}) From aa00e13efa74ed454a5a73aa5f89ef6f0acd1784 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Sun, 2 Sep 2018 22:04:51 -0400 Subject: [PATCH 07/18] change parse_safe(text = x) to parse_safe(x) --- R/geom-label.R | 2 +- R/geom-text.r | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/geom-label.R b/R/geom-label.R index 3a1d3250b5..ac83f4e7f8 100644 --- a/R/geom-label.R +++ b/R/geom-label.R @@ -63,7 +63,7 @@ GeomLabel <- ggproto("GeomLabel", Geom, label.size = 0.25) { lab <- data$label if (parse) { - lab <- parse_safe(text = as.character(lab)) + lab <- parse_safe(as.character(lab)) } data <- coord$transform(data, panel_params) diff --git a/R/geom-text.r b/R/geom-text.r index 88be5c721d..6988c924ea 100644 --- a/R/geom-text.r +++ b/R/geom-text.r @@ -175,7 +175,7 @@ GeomText <- ggproto("GeomText", Geom, na.rm = FALSE, check_overlap = FALSE) { lab <- data$label if (parse) { - lab <- parse_safe(text = as.character(lab)) + lab <- parse_safe(as.character(lab)) } data <- coord$transform(data, panel_params) From 5a21915d33d4c583ea10204defef395049c442eb Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Mon, 3 Sep 2018 11:08:20 -0400 Subject: [PATCH 08/18] modify comment for parse_safe() add URL to GitHub issue #2864 --- R/utilities.r | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/R/utilities.r b/R/utilities.r index d8d214768b..da2979b3dc 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -431,18 +431,15 @@ is_column_vec <- function(x) { length(dims) == 2L && dims[[2]] == 1L } -#' Parse a vector of expressions without silently dropping any items. -#' -#' Note that `parse(text = NULL)` does not work and should be avoided. -#' -#' For more discussion, see #2864 -#' -#' @keywords internal -#' @examples -#' length(parse(text = c("alpha", "", "gamma"))) -#' #> 2 -#' length(parse_safe(text = c("alpha", "", "gamma"))) -#' #> 3 +# Parse takes takes n strings and returns n expressions +# See https://github.com/tidyverse/ggplot2/issues/2864 for discussion +# +# parse(text = c("alpha", "", "gamma")) +# #> expression(alpha, gamma) +# +# parse_safe(text = c("alpha", "", "gamma")) +# #> expression(alpha, NA, gamma) +# parse_safe <- function(text) { out <- vector("expression", length(text)) for (i in seq_along(text)) { From f65ea714fceddb8497c545919b8971c075a71bfe Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Mon, 3 Sep 2018 11:28:02 -0400 Subject: [PATCH 09/18] fix parsing degree labels in geom_sf() --- R/sf.R | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/R/sf.R b/R/sf.R index b2e76abf0b..65d57b206a 100644 --- a/R/sf.R +++ b/R/sf.R @@ -507,6 +507,21 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, graticule$y_start <- sf_rescale01_x(graticule$y_start, y_range) graticule$y_end <- sf_rescale01_x(graticule$y_end, y_range) + # Convert 'degree' to the degree symbol + parse_ids <- grepl("\\bdegree\\b", graticule$degree_label) + if (any(parse_ids)) { + graticule$degree_label <- Map( + function(parse_id, label) { + if (parse_id) { + parse(text = label)[[1]] + } else { + as.expression(label)[[1]] + } + }, + parse_ids, graticule$degree_label + ) + } + list( x_range = x_range, y_range = y_range, @@ -549,18 +564,11 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, graticule <- panel_params$graticule east <- graticule[graticule$type == "E" & !is.na(graticule$degree_label), ] - labs <- east$degree_label - - # parse labels into expressions if required - if (any(grepl("degree", labs))) { - labs <- parse_safe(as.character(labs)) - } - list( top = nullGrob(), bottom = guide_axis( east$x_start, - labs, + east$degree_label, position = "bottom", theme = theme ) @@ -571,17 +579,10 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, graticule <- panel_params$graticule north <- graticule[graticule$type == "N" & !is.na(graticule$degree_label), ] - labs <- north$degree_label - - # parse labels into expressions if required - if (any(grepl("degree", labs))) { - labs <- parse_safe(as.character(labs)) - } - list( left = guide_axis( north$y_start, - labs, + north$degree_label, position = "left", theme = theme ), From f5a853fbf93f60c5864b92d92de7eaffb83de6f6 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Mon, 3 Sep 2018 12:01:28 -0400 Subject: [PATCH 10/18] add news item about parse_safe() --- NEWS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.md b/NEWS.md index f846db73a8..56a652429b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -56,6 +56,11 @@ * `labs()` now has named arguments `title`, `subtitle`, `caption`, and `tag`. Also, `labs()` now accepts tidyeval (@yutannihilation, #2669). +* `geom_text(..., parse = TRUE)` now correctly renders the expected number of + items instead of silently dropping items that are not valid expressions. + This is also fixed for `geom_label()` and the axis labels for `geom_sf()` + (@slowkow, #2867). + # ggplot2 3.0.0 ## Breaking changes From 2a089c09a64b12a3b5eb7fbb9b4536be458dc27d Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Mon, 3 Sep 2018 20:11:33 -0400 Subject: [PATCH 11/18] fix typo --- R/utilities.r | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/utilities.r b/R/utilities.r index da2979b3dc..cb4fa74978 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -431,8 +431,8 @@ is_column_vec <- function(x) { length(dims) == 2L && dims[[2]] == 1L } -# Parse takes takes n strings and returns n expressions -# See https://github.com/tidyverse/ggplot2/issues/2864 for discussion +# Parse takes n strings and returns n expressions. +# See https://github.com/tidyverse/ggplot2/issues/2864 for discussion. # # parse(text = c("alpha", "", "gamma")) # #> expression(alpha, gamma) From 66c92342215e4d61e6a969f5b3ff1e5e6e381662 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Mon, 3 Sep 2018 20:15:14 -0400 Subject: [PATCH 12/18] move parse() into fixup_graticule_labels() --- R/sf.R | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/R/sf.R b/R/sf.R index 65d57b206a..75e33f2ab4 100644 --- a/R/sf.R +++ b/R/sf.R @@ -476,6 +476,21 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, if (!is.null(graticule$plot12)) graticule$degree_label[!graticule$plot12] <- NA + # Convert the string 'degree' to the degree symbol + parse_ids <- grepl("\\bdegree\\b", graticule$degree_label) + if (any(parse_ids)) { + graticule$degree_label <- Map( + function(parse_id, label) { + if (parse_id) { + parse(text = label)[[1]] + } else { + as.expression(label)[[1]] + } + }, + parse_ids, graticule$degree_label + ) + } + graticule }, @@ -507,21 +522,6 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, graticule$y_start <- sf_rescale01_x(graticule$y_start, y_range) graticule$y_end <- sf_rescale01_x(graticule$y_end, y_range) - # Convert 'degree' to the degree symbol - parse_ids <- grepl("\\bdegree\\b", graticule$degree_label) - if (any(parse_ids)) { - graticule$degree_label <- Map( - function(parse_id, label) { - if (parse_id) { - parse(text = label)[[1]] - } else { - as.expression(label)[[1]] - } - }, - parse_ids, graticule$degree_label - ) - } - list( x_range = x_range, y_range = y_range, From 8c798ee30b788f50f2078f4de665df74e90e463c Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Tue, 4 Sep 2018 09:00:57 -0400 Subject: [PATCH 13/18] change description of parse_safe() --- R/utilities.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utilities.r b/R/utilities.r index cb4fa74978..334b8a3a67 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -431,7 +431,7 @@ is_column_vec <- function(x) { length(dims) == 2L && dims[[2]] == 1L } -# Parse takes n strings and returns n expressions. +# Parse takes a vector of n lines and returns m expressions. # See https://github.com/tidyverse/ggplot2/issues/2864 for discussion. # # parse(text = c("alpha", "", "gamma")) From 2b8b45975b1ede5a6517c483e3077d2b078d5a8b Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Tue, 4 Sep 2018 09:06:28 -0400 Subject: [PATCH 14/18] modify news item about parse_safe() --- NEWS.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 1e954d6f67..2ed0d83a50 100644 --- a/NEWS.md +++ b/NEWS.md @@ -61,9 +61,9 @@ (@clauswilke, #2733). * `geom_text(..., parse = TRUE)` now correctly renders the expected number of - items instead of silently dropping items that are not valid expressions. - This is also fixed for `geom_label()` and the axis labels for `geom_sf()` - (@slowkow, #2867). + items instead of silently dropping items that are empty expressions, e.g. + the empty string "". This is also fixed for `geom_label()` and the axis + labels for `geom_sf()` (@slowkow, #2867). # ggplot2 3.0.0 From 637a88e7c7121ef44d9873c908aff3d9d8d05ebc Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Tue, 4 Sep 2018 09:07:51 -0400 Subject: [PATCH 15/18] add stopifnot() to parse_safe() --- R/utilities.r | 1 + 1 file changed, 1 insertion(+) diff --git a/R/utilities.r b/R/utilities.r index 334b8a3a67..955e2d8ff9 100644 --- a/R/utilities.r +++ b/R/utilities.r @@ -441,6 +441,7 @@ is_column_vec <- function(x) { # #> expression(alpha, NA, gamma) # parse_safe <- function(text) { + stopifnot(is.character(text)) out <- vector("expression", length(text)) for (i in seq_along(text)) { expr <- parse(text = text[[i]]) From 95025cb2511bfba731496c1578da17178b50909c Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Tue, 4 Sep 2018 10:20:38 -0400 Subject: [PATCH 16/18] simplify parsing code in sf.R --- R/sf.R | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/R/sf.R b/R/sf.R index 75e33f2ab4..abcc60bceb 100644 --- a/R/sf.R +++ b/R/sf.R @@ -477,18 +477,11 @@ CoordSf <- ggproto("CoordSf", CoordCartesian, graticule$degree_label[!graticule$plot12] <- NA # Convert the string 'degree' to the degree symbol - parse_ids <- grepl("\\bdegree\\b", graticule$degree_label) - if (any(parse_ids)) { - graticule$degree_label <- Map( - function(parse_id, label) { - if (parse_id) { - parse(text = label)[[1]] - } else { - as.expression(label)[[1]] - } - }, - parse_ids, graticule$degree_label - ) + has_degree <- grepl("\\bdegree\\b", graticule$degree_label) + if (any(has_degree)) { + labels <- as.list(graticule$degree_label) + labels[has_degree] <- parse_safe(graticule$degree_label[has_degree]) + graticule$degree_label <- labels } graticule From 54d87c7fae26680435972a09f39341beddf977f7 Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Tue, 4 Sep 2018 10:24:09 -0400 Subject: [PATCH 17/18] improve news item about parse_safe() --- NEWS.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 2ed0d83a50..7ec8955346 100644 --- a/NEWS.md +++ b/NEWS.md @@ -62,8 +62,9 @@ * `geom_text(..., parse = TRUE)` now correctly renders the expected number of items instead of silently dropping items that are empty expressions, e.g. - the empty string "". This is also fixed for `geom_label()` and the axis - labels for `geom_sf()` (@slowkow, #2867). + the empty string "". If an expression spans multiple lines, we take just + the first line and drop the rest. This same issue is also fixed for + `geom_label()` and the axis labels for `geom_sf()` (@slowkow, #2867). # ggplot2 3.0.0 From 06150624620faf4d57b50e729f6a1cb2b1b1439c Mon Sep 17 00:00:00 2001 From: Kamil Slowikowski Date: Tue, 4 Sep 2018 10:40:54 -0400 Subject: [PATCH 18/18] do not test parse_safe(factor(...)) `parse_safe()` takes a character vector, not a factor --- tests/testthat/test-utilities.r | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/testthat/test-utilities.r b/tests/testthat/test-utilities.r index a98c6052b1..7ac5b32e3b 100644 --- a/tests/testthat/test-utilities.r +++ b/tests/testthat/test-utilities.r @@ -64,11 +64,6 @@ test_that("parse_safe works with simple expressions", { parse_safe(c(NA, "a", NA, "alpha")), expression(NA, a, NA, alpha) ) - - expect_equal( - parse_safe(factor(c("alpha", "beta", ""))), - expression(2, 3, 1) - ) }) test_that("parse_safe works with multi expressions", {