From a21baea54cb5cacf3f7c11ff065abe2c46fbd0b1 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Mon, 5 Nov 2018 23:16:44 +0900 Subject: [PATCH 01/12] Add a failing test about labelling --- tests/testthat/test-aes-calculated.r | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/testthat/test-aes-calculated.r b/tests/testthat/test-aes-calculated.r index f52f319845..7c8b00b963 100644 --- a/tests/testthat/test-aes-calculated.r +++ b/tests/testthat/test-aes-calculated.r @@ -28,3 +28,7 @@ test_that("calculation stripped from labels", { expect_equal(make_labels(aes(x = ..y..)), list(x = "y")) expect_equal(make_labels(aes(x = stat(y))), list(x = "y")) }) + +test_that("make_labels() deprases variable without backticks", { + expect_equal(make_labels(aes(x = `a b`)), list(x = "a b")) +}) From e9fe0edbaaccead2a66d46fb82cb9f0b7daae111 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Mon, 5 Nov 2018 23:17:32 +0900 Subject: [PATCH 02/12] Use quo_name() for making labels --- R/aes-calculated.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/aes-calculated.r b/R/aes-calculated.r index 4e16cfe4e6..7becc76d20 100644 --- a/R/aes-calculated.r +++ b/R/aes-calculated.r @@ -91,7 +91,7 @@ make_labels <- function(mapping) { if (is.atomic(mapping)) { aesthetic } else { - x <- rlang::quo_text(strip_dots(mapping)) + x <- rlang::quo_name(strip_dots(mapping)) if (length(x) > 1) { x <- paste0(x[[1]], "...") } From c9c3b0c35b24fb078fc6f1b9789190cc2a6c666e Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 6 Nov 2018 21:06:42 +0900 Subject: [PATCH 03/12] Add more tests about make_labels() --- tests/testthat/test-aes-calculated.r | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-aes-calculated.r b/tests/testthat/test-aes-calculated.r index 7c8b00b963..8229cc7bce 100644 --- a/tests/testthat/test-aes-calculated.r +++ b/tests/testthat/test-aes-calculated.r @@ -24,11 +24,16 @@ test_that("strip_dots remove dots around calculated aesthetics", { ) }) -test_that("calculation stripped from labels", { - expect_equal(make_labels(aes(x = ..y..)), list(x = "y")) - expect_equal(make_labels(aes(x = stat(y))), list(x = "y")) -}) +test_that("make_labels() deprases mappings properly", { + # calculation stripped from labels + expect_identical(make_labels(aes(x = ..y..)), list(x = "y")) + expect_identical(make_labels(aes(x = stat(y))), list(x = "y")) -test_that("make_labels() deprases variable without backticks", { - expect_equal(make_labels(aes(x = `a b`)), list(x = "a b")) + # symbol is always deparsed without backticks + expect_identical(make_labels(aes(x = `a b`)), list(x = "a b")) + # long expression is abbreviated with ... + expect_identical(make_labels(aes(x = 2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * x)), + list(x = "2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * ...")) + # if the mapping is a literal, the aesthetics is used + expect_identical(make_labels(aes(x = 1)), list(x = "x")) }) From 2f3fffd97f4c62bee8db29d7bf9be2db232b8e2b Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 6 Nov 2018 21:07:30 +0900 Subject: [PATCH 04/12] Improve make_labels() --- R/aes-calculated.r | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/R/aes-calculated.r b/R/aes-calculated.r index 7becc76d20..cadf1ae2b4 100644 --- a/R/aes-calculated.r +++ b/R/aes-calculated.r @@ -89,14 +89,17 @@ make_labels <- function(mapping) { default_label <- function(aesthetic, mapping) { # e.g., geom_smooth(aes(colour = "loess")) if (is.atomic(mapping)) { - aesthetic + return(aesthetic) + } + + mapping <- strip_dots(mapping) + if (rlang::is_quosure(mapping) && rlang::quo_is_symbol(mapping)) { + name <- rlang::as_string(rlang::quo_get_expr(mapping)) } else { - x <- rlang::quo_name(strip_dots(mapping)) - if (length(x) > 1) { - x <- paste0(x[[1]], "...") - } - x + name <- rlang::quo_text(mapping) + name <- gsub("\n.*$", "...", name) } + name } Map(default_label, names(mapping), mapping) } From bec320fee3982bcf749e26d7414c4c9307a4db61 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 6 Nov 2018 21:14:11 +0900 Subject: [PATCH 05/12] Add a case when the mapping is NULL --- R/aes-calculated.r | 2 +- tests/testthat/test-aes-calculated.r | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/R/aes-calculated.r b/R/aes-calculated.r index cadf1ae2b4..ff93d8c86e 100644 --- a/R/aes-calculated.r +++ b/R/aes-calculated.r @@ -87,7 +87,7 @@ strip_dots <- function(expr) { # Convert aesthetic mapping into text labels make_labels <- function(mapping) { default_label <- function(aesthetic, mapping) { - # e.g., geom_smooth(aes(colour = "loess")) + # e.g., geom_smooth(aes(colour = "loess")) or aes(y = NULL) if (is.atomic(mapping)) { return(aesthetic) } diff --git a/tests/testthat/test-aes-calculated.r b/tests/testthat/test-aes-calculated.r index 8229cc7bce..d396098a7b 100644 --- a/tests/testthat/test-aes-calculated.r +++ b/tests/testthat/test-aes-calculated.r @@ -34,6 +34,7 @@ test_that("make_labels() deprases mappings properly", { # long expression is abbreviated with ... expect_identical(make_labels(aes(x = 2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * x)), list(x = "2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * ...")) - # if the mapping is a literal, the aesthetics is used + # if the mapping is a literal or NULL, the aesthetics is used expect_identical(make_labels(aes(x = 1)), list(x = "x")) + expect_identical(make_labels(aes(x = NULL)), list(x = "x")) }) From 2552f9509513a3500b4e14e6b3bd96ab90f53e70 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 6 Nov 2018 21:15:01 +0900 Subject: [PATCH 06/12] Use make_labels() in ggplot_add.uneval() as well --- R/plot-construction.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/plot-construction.r b/R/plot-construction.r index 8a65b58e14..c8ebca0378 100644 --- a/R/plot-construction.r +++ b/R/plot-construction.r @@ -123,7 +123,7 @@ ggplot_add.uneval <- function(object, plot, object_name) { # defaults() doesn't copy class, so copy it. class(plot$mapping) <- class(object) - labels <- lapply(object, function(x) if (is.null(x)) x else rlang::quo_name(x)) + labels <- make_labels(object) names(labels) <- names(object) update_labels(plot, labels) } From f75440b71f470f389bcba52a0851ce853b501a9a Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Tue, 6 Nov 2018 21:57:49 +0900 Subject: [PATCH 07/12] Return NULL if the mapping is NULL --- R/aes-calculated.r | 6 +++++- tests/testthat/test-aes-calculated.r | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/R/aes-calculated.r b/R/aes-calculated.r index ff93d8c86e..1d789c4701 100644 --- a/R/aes-calculated.r +++ b/R/aes-calculated.r @@ -87,7 +87,11 @@ strip_dots <- function(expr) { # Convert aesthetic mapping into text labels make_labels <- function(mapping) { default_label <- function(aesthetic, mapping) { - # e.g., geom_smooth(aes(colour = "loess")) or aes(y = NULL) + if (is.null(mapping)) { + return(NULL) + } + + # e.g., geom_smooth(aes(colour = "loess")) if (is.atomic(mapping)) { return(aesthetic) } diff --git a/tests/testthat/test-aes-calculated.r b/tests/testthat/test-aes-calculated.r index d396098a7b..ec5228b033 100644 --- a/tests/testthat/test-aes-calculated.r +++ b/tests/testthat/test-aes-calculated.r @@ -34,7 +34,8 @@ test_that("make_labels() deprases mappings properly", { # long expression is abbreviated with ... expect_identical(make_labels(aes(x = 2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * x)), list(x = "2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * ...")) - # if the mapping is a literal or NULL, the aesthetics is used + # if the mapping is a literal, the aesthetics is used expect_identical(make_labels(aes(x = 1)), list(x = "x")) - expect_identical(make_labels(aes(x = NULL)), list(x = "x")) + # if the mapping is NULL, there will be no labels + expect_identical(make_labels(aes(x = NULL)), list(x = NULL)) }) From 71c00b2577bc57b7fc530053fa9c20384a10d224 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Wed, 7 Nov 2018 01:11:50 +0900 Subject: [PATCH 08/12] Revert "Return NULL if the mapping is NULL" This reverts commit f75440b71f470f389bcba52a0851ce853b501a9a. --- R/aes-calculated.r | 6 +----- tests/testthat/test-aes-calculated.r | 5 ++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/R/aes-calculated.r b/R/aes-calculated.r index 1d789c4701..ff93d8c86e 100644 --- a/R/aes-calculated.r +++ b/R/aes-calculated.r @@ -87,11 +87,7 @@ strip_dots <- function(expr) { # Convert aesthetic mapping into text labels make_labels <- function(mapping) { default_label <- function(aesthetic, mapping) { - if (is.null(mapping)) { - return(NULL) - } - - # e.g., geom_smooth(aes(colour = "loess")) + # e.g., geom_smooth(aes(colour = "loess")) or aes(y = NULL) if (is.atomic(mapping)) { return(aesthetic) } diff --git a/tests/testthat/test-aes-calculated.r b/tests/testthat/test-aes-calculated.r index ec5228b033..d396098a7b 100644 --- a/tests/testthat/test-aes-calculated.r +++ b/tests/testthat/test-aes-calculated.r @@ -34,8 +34,7 @@ test_that("make_labels() deprases mappings properly", { # long expression is abbreviated with ... expect_identical(make_labels(aes(x = 2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * x)), list(x = "2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * ...")) - # if the mapping is a literal, the aesthetics is used + # if the mapping is a literal or NULL, the aesthetics is used expect_identical(make_labels(aes(x = 1)), list(x = "x")) - # if the mapping is NULL, there will be no labels - expect_identical(make_labels(aes(x = NULL)), list(x = NULL)) + expect_identical(make_labels(aes(x = NULL)), list(x = "x")) }) From dde60939edcc5d6897bebcfa63a59d19a702510e Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Wed, 7 Nov 2018 01:13:27 +0900 Subject: [PATCH 09/12] Tweak a test --- tests/testthat/test-aes.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-aes.r b/tests/testthat/test-aes.r index fd36cbc1f0..0ce7c2e43e 100644 --- a/tests/testthat/test-aes.r +++ b/tests/testthat/test-aes.r @@ -95,7 +95,7 @@ test_that("quosures are squashed when creating default label for a mapping", { test_that("labelling doesn't cause error if aesthetic is NULL", { p <- ggplot(mtcars) + aes(x = NULL) - expect_null(p$labels$x) + expect_identical(p$labels$x, "x") }) test_that("aes standardises aesthetic names", { From 1a96032ee6922f77273623b864bb67a4c40dbc6c Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Sun, 11 Nov 2018 11:13:23 +0900 Subject: [PATCH 10/12] Soften the test for a long expression --- tests/testthat/test-aes-calculated.r | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-aes-calculated.r b/tests/testthat/test-aes-calculated.r index d396098a7b..8e97dd3ff7 100644 --- a/tests/testthat/test-aes-calculated.r +++ b/tests/testthat/test-aes-calculated.r @@ -32,8 +32,9 @@ test_that("make_labels() deprases mappings properly", { # symbol is always deparsed without backticks expect_identical(make_labels(aes(x = `a b`)), list(x = "a b")) # long expression is abbreviated with ... - expect_identical(make_labels(aes(x = 2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * x)), - list(x = "2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * ...")) + x_lab <- make_labels(aes(x = 2 * x * exp(`coef 1` * x^2) * 2 * x * exp(`coef 1` * x^2) * 2 * x))$x + expect_length(x_lab, 1L) + expect_match(x_lab, "...$") # if the mapping is a literal or NULL, the aesthetics is used expect_identical(make_labels(aes(x = 1)), list(x = "x")) expect_identical(make_labels(aes(x = NULL)), list(x = "x")) From 8d7bb60c3b849d3d4e4908bf1318e72e95f3fd45 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Sun, 11 Nov 2018 11:34:38 +0900 Subject: [PATCH 11/12] Add a news bullet --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index 5f528c2b00..1c56979466 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # ggplot2 3.1.0.9000 +* Default labels are now generated more consistently; e.g., symbols no longer + get backticks, and long expressions are abreviated with `...` + (@yutannihilation, #2981). + # ggplot2 3.1.0 ## Breaking changes From 64529db32df1a91ad74ffa82735d2b447ce1ab06 Mon Sep 17 00:00:00 2001 From: Hiroaki Yutani Date: Sun, 11 Nov 2018 11:36:06 +0900 Subject: [PATCH 12/12] Fix a typo --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 1c56979466..41568e171e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,7 @@ # ggplot2 3.1.0.9000 * Default labels are now generated more consistently; e.g., symbols no longer - get backticks, and long expressions are abreviated with `...` + get backticks, and long expressions are abbreviated with `...` (@yutannihilation, #2981). # ggplot2 3.1.0