From 0cec41d687ee14e909b2091bc17f617701ff74c7 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Wed, 19 Jun 2019 10:45:21 -0400 Subject: [PATCH 01/11] add visual test cases for basic axis drawing --- tests/figs/guides/axis-guides-basic.svg | 84 +++++++++++++++++++ tests/figs/guides/axis-guides-zero-breaks.svg | 60 +++++++++++++ tests/testthat/test-guides.R | 30 +++++++ 3 files changed, 174 insertions(+) create mode 100644 tests/figs/guides/axis-guides-basic.svg create mode 100644 tests/figs/guides/axis-guides-zero-breaks.svg diff --git a/tests/figs/guides/axis-guides-basic.svg b/tests/figs/guides/axis-guides-basic.svg new file mode 100644 index 0000000000..efc918fbba --- /dev/null +++ b/tests/figs/guides/axis-guides-basic.svg @@ -0,0 +1,84 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1 +2 +3 + + + + + + + +1 +2 +3 + + + + +1 +2 +3 + +1 +2 +3 + + + + diff --git a/tests/figs/guides/axis-guides-zero-breaks.svg b/tests/figs/guides/axis-guides-zero-breaks.svg new file mode 100644 index 0000000000..b157ad8d9b --- /dev/null +++ b/tests/figs/guides/axis-guides-zero-breaks.svg @@ -0,0 +1,60 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 1800a87cbe..dbd1a1dd58 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -50,6 +50,36 @@ test_that("show.legend handles named vectors", { # Visual tests ------------------------------------------------------------ test_that("axis guides are drawn correctly", { + theme_test_axis <- theme_test() + theme(axis.line = element_line(size = 0.5)) + test_draw_axis <- function(n_breaks = 3, + break_labels = as.character, + positions = c("top", "right", "bottom", "left"), + theme = theme_test_axis, + ...) { + + break_positions <- seq_len(n_breaks) / (n_breaks + 1) + break_labels <- break_labels(seq_len(n_breaks)) + + # create the axes + axes <- lapply(positions, function(position) { + draw_axis(break_positions, break_labels, axis_position = position, theme = theme, ...) + }) + axes_grob <- gTree(children = do.call(gList, axes)) + + # arrange them so there's some padding on each side + gt <- gtable( + widths = unit(c(0.05, 0.9, 0.05), "npc"), + heights = unit(c(0.05, 0.9, 0.05), "npc") + ) + gt <- gtable_add_grob(gt, list(axes_grob), 2, 2, clip = "off") + plot(gt) + } + + expect_doppelganger("axis guides basic", test_draw_axis) + expect_doppelganger("axis guides, zero breaks", function() test_draw_axis(n_breaks = 0)) +}) + +test_that("axis guides are drawn correctly in plots", { expect_doppelganger("align facet labels, facets horizontal", qplot(hwy, reorder(model, hwy), data = mpg) + facet_grid(manufacturer ~ ., scales = "free", space = "free") + From 0a31ad798a0c8e0cca8234260206a72c9f8267f7 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Wed, 19 Jun 2019 11:25:48 -0400 Subject: [PATCH 02/11] add check.overlap option for overlapping axis labels --- R/guides-axis.r | 39 +++- R/margins.R | 10 +- R/theme-elements.r | 2 +- .../figs/guides/axis-guides-check-overlap.svg | 198 ++++++++++++++++++ tests/testthat/test-guides.R | 18 +- 5 files changed, 258 insertions(+), 9 deletions(-) create mode 100644 tests/figs/guides/axis-guides-check-overlap.svg diff --git a/R/guides-axis.r b/R/guides-axis.r index 62c6f29b1c..b8563898ba 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -4,11 +4,12 @@ #' @param break_position position of ticks #' @param break_labels labels at ticks #' @param axis_position position of axis (top, bottom, left or right) -#' @param theme A [theme()] object +#' @param theme A complete [theme()] object #' #' @noRd #' -draw_axis <- function(break_positions, break_labels, axis_position, theme) { +draw_axis <- function(break_positions, break_labels, axis_position, theme, + check.overlap = FALSE) { axis_position <- match.arg(axis_position, c("top", "bottom", "right", "left")) aesthetic <- if (axis_position %in% c("top", "bottom")) "x" else "y" @@ -80,11 +81,18 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme) { } } + if (check.overlap) { + priority <- axis_label_overlap_priority(n_breaks) + break_labels <- break_labels[priority] + break_positions <- break_positions[priority] + } + labels_grob <- exec( element_grob, label_element, !!position_dim := unit(break_positions, "native"), !!label_margin_name := TRUE, - label = break_labels + label = break_labels, + check.overlap = check.overlap ) ticks_grob <- exec( @@ -124,3 +132,28 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme) { vp = justvp ) } + +#' Deterine the label priority for a given number of labels +#' +#' @param n The number of labels +#' +#' @return The vector `seq_len(n)` arranged such that the +#' first, last, and middle elements are recursively +#' placed at the beginning of the vector. +#' @noRd +#' +axis_label_overlap_priority <- function(n) { + if(n <= 0) return(numeric(0)) + + first <- 1 + last <- n + middle <- (n + 1) %/% 2 + + order <- c( + first, last, middle, + first + axis_label_overlap_priority(middle - first - 1), + middle + axis_label_overlap_priority(last - middle - 1) + ) + + unique(order) +} diff --git a/R/margins.R b/R/margins.R index 6314981f6a..b85f37a5fe 100644 --- a/R/margins.R +++ b/R/margins.R @@ -37,7 +37,7 @@ margin_width <- function(grob, margins) { #' #' @noRd title_spec <- function(label, x, y, hjust, vjust, angle, gp = gpar(), - debug = FALSE) { + debug = FALSE, check.overlap = FALSE) { if (is.null(label)) return(zeroGrob()) @@ -56,7 +56,8 @@ title_spec <- function(label, x, y, hjust, vjust, angle, gp = gpar(), hjust = hjust, vjust = vjust, rot = angle, - gp = gp + gp = gp, + check.overlap = check.overlap ) # The grob dimensions don't include the text descenders, so these need to be added @@ -175,7 +176,7 @@ add_margins <- function(grob, height, width, margin = NULL, #' @noRd titleGrob <- function(label, x, y, hjust, vjust, angle = 0, gp = gpar(), margin = NULL, margin_x = FALSE, margin_y = FALSE, - debug = FALSE) { + debug = FALSE, check.overlap = FALSE) { if (is.null(label)) return(zeroGrob()) @@ -189,7 +190,8 @@ titleGrob <- function(label, x, y, hjust, vjust, angle = 0, gp = gpar(), vjust = vjust, angle = angle, gp = gp, - debug = debug + debug = debug, + check.overlap = check.overlap ) add_margins( diff --git a/R/theme-elements.r b/R/theme-elements.r index 247c4c868c..f2cdd5c0fa 100644 --- a/R/theme-elements.r +++ b/R/theme-elements.r @@ -215,7 +215,7 @@ element_grob.element_text <- function(element, label = "", x = NULL, y = NULL, titleGrob(label, x, y, hjust = hj, vjust = vj, angle = angle, gp = modify_list(element_gp, gp), margin = margin, - margin_x = margin_x, margin_y = margin_y, debug = element$debug) + margin_x = margin_x, margin_y = margin_y, debug = element$debug, ...) } diff --git a/tests/figs/guides/axis-guides-check-overlap.svg b/tests/figs/guides/axis-guides-check-overlap.svg new file mode 100644 index 0000000000..ee0de7d535 --- /dev/null +++ b/tests/figs/guides/axis-guides-check-overlap.svg @@ -0,0 +1,198 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000,000,000 +20,000,000,000 +10,000,000,000 +5,000,000,000 +3,000,000,000 +8,000,000,000 +15,000,000,000 +12,000,000,000 +18,000,000,000 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000,000,000 +20,000,000,000 +10,000,000,000 +2,000,000,000 +9,000,000,000 +5,000,000,000 +3,000,000,000 +4,000,000,000 +6,000,000,000 +8,000,000,000 +7,000,000,000 +11,000,000,000 +19,000,000,000 +15,000,000,000 +12,000,000,000 +14,000,000,000 +13,000,000,000 +16,000,000,000 +18,000,000,000 +17,000,000,000 + + + + + + + + + + + + + + + + + + + + + +1,000,000,000 +20,000,000,000 +10,000,000,000 +5,000,000,000 +3,000,000,000 +8,000,000,000 +15,000,000,000 +12,000,000,000 +18,000,000,000 + +1,000,000,000 +20,000,000,000 +10,000,000,000 +2,000,000,000 +9,000,000,000 +5,000,000,000 +3,000,000,000 +4,000,000,000 +6,000,000,000 +8,000,000,000 +7,000,000,000 +11,000,000,000 +19,000,000,000 +15,000,000,000 +12,000,000,000 +14,000,000,000 +13,000,000,000 +16,000,000,000 +18,000,000,000 +17,000,000,000 + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index dbd1a1dd58..c193ed7e25 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -46,6 +46,13 @@ test_that("show.legend handles named vectors", { expect_equal(n_legends(p), 0) }) +test_that("axis_label_overlap_priority always returns the correct number of elements", { + expect_identical(axis_label_overlap_priority(0), numeric(0)) + expect_setequal(axis_label_overlap_priority(1), seq_len(1)) + expect_setequal(axis_label_overlap_priority(5), seq_len(5)) + expect_setequal(axis_label_overlap_priority(10), seq_len(10)) + expect_setequal(axis_label_overlap_priority(100), seq_len(100)) +}) # Visual tests ------------------------------------------------------------ @@ -75,8 +82,17 @@ test_that("axis guides are drawn correctly", { plot(gt) } - expect_doppelganger("axis guides basic", test_draw_axis) + # basic + expect_doppelganger("axis guides basic", function() test_draw_axis()) expect_doppelganger("axis guides, zero breaks", function() test_draw_axis(n_breaks = 0)) + + # long label strategies + long_labels <- function(b) comma(b * 1e9) + expect_doppelganger( + "axis guides, check overlap", + function() test_draw_axis(20, long_labels, check.overlap = TRUE) + ) + }) test_that("axis guides are drawn correctly in plots", { From 803aa08d1dddf966a95247882ec2224e5c284885 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Wed, 19 Jun 2019 13:57:55 -0400 Subject: [PATCH 03/11] add axis rotation to default axis guide --- R/guides-axis.r | 60 +++++++- .../guides/axis-guides-negative-rotation.svg | 140 ++++++++++++++++++ .../guides/axis-guides-positive-rotation.svg | 140 ++++++++++++++++++ ...axis-guides-vertical-negative-rotation.svg | 140 ++++++++++++++++++ .../guides/axis-guides-vertical-rotation.svg | 140 ++++++++++++++++++ .../figs/guides/axis-guides-zero-rotation.svg | 140 ++++++++++++++++++ tests/testthat/test-guides.R | 40 ++++- 7 files changed, 793 insertions(+), 7 deletions(-) create mode 100644 tests/figs/guides/axis-guides-negative-rotation.svg create mode 100644 tests/figs/guides/axis-guides-positive-rotation.svg create mode 100644 tests/figs/guides/axis-guides-vertical-negative-rotation.svg create mode 100644 tests/figs/guides/axis-guides-vertical-rotation.svg create mode 100644 tests/figs/guides/axis-guides-zero-rotation.svg diff --git a/R/guides-axis.r b/R/guides-axis.r index b8563898ba..e24df7465b 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -9,7 +9,7 @@ #' @noRd #' draw_axis <- function(break_positions, break_labels, axis_position, theme, - check.overlap = FALSE) { + check.overlap = FALSE, angle = NULL) { axis_position <- match.arg(axis_position, c("top", "bottom", "right", "left")) aesthetic <- if (axis_position %in% c("top", "bottom")) "x" else "y" @@ -25,6 +25,14 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, tick_length <- calc_element(tick_length_element_name, theme) label_element <- calc_element(label_element_name, theme) + # override label element parameters for rotation + if (inherits(label_element, "element_text")) { + label_element <- merge_element( + axis_label_element_overrides(axis_position, angle), + label_element + ) + } + # conditionally set parameters that depend on axis orientation is_vertical <- axis_position %in% c("left", "right") @@ -143,7 +151,7 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, #' @noRd #' axis_label_overlap_priority <- function(n) { - if(n <= 0) return(numeric(0)) + if (n <= 0) return(numeric(0)) first <- 1 last <- n @@ -157,3 +165,51 @@ axis_label_overlap_priority <- function(n) { unique(order) } + +#' Override axis text angle and alignment +#' +#' @param axis_position One of bottom, left, top, or right +#' @param angle The text angle, or NULL to override nothing +#' +#' @return An [element_text()] that contains parameters that should be +#' overridden from the user- or theme-supplied element. +#' @noRd +#' +axis_label_element_overrides <- function(axis_position, angle = NULL) { + if (is.null(angle)) { + return(element_text(angle = NULL, hjust = NULL, vjust = NULL)) + } + + # it is not worth the effort to align upside-down labels properly + if (angle > 90 || angle < -90) { + stop("`angle` must be between 90 and -90", call. = FALSE) + } + + if (axis_position == "bottom") { + element_text( + angle = angle, + hjust = if (angle > 0) 1 else if (angle < 0) 0 else 0.5, + vjust = if (abs(angle) == 90) 0.5 else 1 + ) + } else if (axis_position == "left") { + element_text( + angle = angle, + hjust = if (abs(angle) == 90) 0.5 else 1, + vjust = if (angle > 0) 0 else if (angle < 0) 1 else 0.5, + ) + } else if (axis_position == "top") { + element_text( + angle = angle, + hjust = if (angle > 0) 0 else if (angle < 0) 1 else 0.5, + vjust = if (abs(angle) == 90) 0.5 else 0 + ) + } else if (axis_position == "right") { + element_text( + angle = angle, + hjust = if (abs(angle) == 90) 0.5 else 0, + vjust = if (angle > 0) 1 else if (angle < 0) 0 else 0.5, + ) + } else { + stop("Unrecognized position: '", axis_position, "'", call. = FALSE) + } +} diff --git a/tests/figs/guides/axis-guides-negative-rotation.svg b/tests/figs/guides/axis-guides-negative-rotation.svg new file mode 100644 index 0000000000..fb9c1da5d3 --- /dev/null +++ b/tests/figs/guides/axis-guides-negative-rotation.svg @@ -0,0 +1,140 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + diff --git a/tests/figs/guides/axis-guides-positive-rotation.svg b/tests/figs/guides/axis-guides-positive-rotation.svg new file mode 100644 index 0000000000..fe73c5aa16 --- /dev/null +++ b/tests/figs/guides/axis-guides-positive-rotation.svg @@ -0,0 +1,140 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + diff --git a/tests/figs/guides/axis-guides-vertical-negative-rotation.svg b/tests/figs/guides/axis-guides-vertical-negative-rotation.svg new file mode 100644 index 0000000000..ade52eeafd --- /dev/null +++ b/tests/figs/guides/axis-guides-vertical-negative-rotation.svg @@ -0,0 +1,140 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + diff --git a/tests/figs/guides/axis-guides-vertical-rotation.svg b/tests/figs/guides/axis-guides-vertical-rotation.svg new file mode 100644 index 0000000000..66f71a4725 --- /dev/null +++ b/tests/figs/guides/axis-guides-vertical-rotation.svg @@ -0,0 +1,140 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + diff --git a/tests/figs/guides/axis-guides-zero-rotation.svg b/tests/figs/guides/axis-guides-zero-rotation.svg new file mode 100644 index 0000000000..e3d93e9a3c --- /dev/null +++ b/tests/figs/guides/axis-guides-zero-rotation.svg @@ -0,0 +1,140 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + +1,000 +2,000 +3,000 +4,000 +5,000 +6,000 +7,000 +8,000 +9,000 +10,000 + + + + + + + + + + + diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index c193ed7e25..8e871952c2 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -54,18 +54,24 @@ test_that("axis_label_overlap_priority always returns the correct number of elem expect_setequal(axis_label_overlap_priority(100), seq_len(100)) }) +test_that("axis_label_element_overrides errors when angles are outside the range [0, 90]", { + expect_is(axis_label_element_overrides("bottom", 0), "element") + expect_error(axis_label_element_overrides("bottom", 91), "`angle` must") + expect_error(axis_label_element_overrides("bottom", -91), "`angle` must") +}) + # Visual tests ------------------------------------------------------------ test_that("axis guides are drawn correctly", { theme_test_axis <- theme_test() + theme(axis.line = element_line(size = 0.5)) test_draw_axis <- function(n_breaks = 3, - break_labels = as.character, + labels = as.character, positions = c("top", "right", "bottom", "left"), theme = theme_test_axis, ...) { break_positions <- seq_len(n_breaks) / (n_breaks + 1) - break_labels <- break_labels(seq_len(n_breaks)) + break_labels <- labels(seq_len(n_breaks)) # create the axes axes <- lapply(positions, function(position) { @@ -86,13 +92,37 @@ test_that("axis guides are drawn correctly", { expect_doppelganger("axis guides basic", function() test_draw_axis()) expect_doppelganger("axis guides, zero breaks", function() test_draw_axis(n_breaks = 0)) - # long label strategies - long_labels <- function(b) comma(b * 1e9) + # overlapping text expect_doppelganger( "axis guides, check overlap", - function() test_draw_axis(20, long_labels, check.overlap = TRUE) + function() test_draw_axis(20, labels = function(b) comma(b * 1e9), check.overlap = TRUE) + ) + + # rotated text + expect_doppelganger( + "axis guides, zero rotation", + function() test_draw_axis(10, labels = function(b) comma(b * 1e3), angle = 0) + ) + + expect_doppelganger( + "axis guides, positive rotation", + function() test_draw_axis(10, labels = function(b) comma(b * 1e3), angle = 45) ) + expect_doppelganger( + "axis guides, negative rotation", + function() test_draw_axis(10, labels = function(b) comma(b * 1e3), angle = -45) + ) + + expect_doppelganger( + "axis guides, vertical rotation", + function() test_draw_axis(10, labels = function(b) comma(b * 1e3), angle = 90) + ) + + expect_doppelganger( + "axis guides, vertical negative rotation", + function() test_draw_axis(10, labels = function(b) comma(b * 1e3), angle = -90) + ) }) test_that("axis guides are drawn correctly in plots", { From 41fe45f66b2f4f8ab00ad318f1c32ef5fe07ede6 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Wed, 19 Jun 2019 14:59:02 -0400 Subject: [PATCH 04/11] fix priority algorithm, document new parameters --- R/guides-axis.r | 35 ++++--- .../figs/guides/axis-guides-check-overlap.svg | 92 +++++++++---------- tests/testthat/test-guides.R | 10 +- 3 files changed, 74 insertions(+), 63 deletions(-) diff --git a/R/guides-axis.r b/R/guides-axis.r index e24df7465b..89b94aff9d 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -5,6 +5,11 @@ #' @param break_labels labels at ticks #' @param axis_position position of axis (top, bottom, left or right) #' @param theme A complete [theme()] object +#' @param check.overlap silently remove overlapping labels, +#' (recursively) prioritizing the first, last, and middle labels. +#' @param angle The angle at which the text should be rotated (between +#' -90 and 90), or `NULL` if the angle should be obtained from the +#' `theme`. #' #' @noRd #' @@ -90,7 +95,7 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, } if (check.overlap) { - priority <- axis_label_overlap_priority(n_breaks) + priority <- axis_label_priority(n_breaks) break_labels <- break_labels[priority] break_positions <- break_positions[priority] } @@ -150,20 +155,26 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, #' placed at the beginning of the vector. #' @noRd #' -axis_label_overlap_priority <- function(n) { - if (n <= 0) return(numeric(0)) +axis_label_priority <- function(n) { + if (n <= 0) { + return(numeric(0)) + } - first <- 1 - last <- n - middle <- (n + 1) %/% 2 + c(1, n, axis_label_priority_between(1, n)) +} - order <- c( - first, last, middle, - first + axis_label_overlap_priority(middle - first - 1), - middle + axis_label_overlap_priority(last - middle - 1) - ) +axis_label_priority_between <- function(x, y) { + n <- y - x + 1 + if (n <= 2) { + return(numeric(0)) + } - unique(order) + mid <- x - 1 + (n + 1) %/% 2 + c( + mid, + axis_label_priority_between(x, mid), + axis_label_priority_between(mid, y) + ) } #' Override axis text angle and alignment diff --git a/tests/figs/guides/axis-guides-check-overlap.svg b/tests/figs/guides/axis-guides-check-overlap.svg index ee0de7d535..35615f5d63 100644 --- a/tests/figs/guides/axis-guides-check-overlap.svg +++ b/tests/figs/guides/axis-guides-check-overlap.svg @@ -59,140 +59,140 @@ 10,000,000,000 5,000,000,000 3,000,000,000 -8,000,000,000 +7,000,000,000 15,000,000,000 12,000,000,000 -18,000,000,000 +17,000,000,000 - - + + - - - + - + + + - + - - + + - - - + - + + + - + 1,000,000,000 20,000,000,000 10,000,000,000 -2,000,000,000 -9,000,000,000 5,000,000,000 3,000,000,000 +2,000,000,000 4,000,000,000 +7,000,000,000 6,000,000,000 8,000,000,000 -7,000,000,000 -11,000,000,000 -19,000,000,000 +9,000,000,000 15,000,000,000 12,000,000,000 -14,000,000,000 +11,000,000,000 13,000,000,000 +14,000,000,000 +17,000,000,000 16,000,000,000 18,000,000,000 -17,000,000,000 +19,000,000,000 - - + + - - - + - + + + - + 1,000,000,000 20,000,000,000 10,000,000,000 5,000,000,000 3,000,000,000 -8,000,000,000 +7,000,000,000 15,000,000,000 12,000,000,000 -18,000,000,000 +17,000,000,000 1,000,000,000 20,000,000,000 10,000,000,000 -2,000,000,000 -9,000,000,000 5,000,000,000 3,000,000,000 +2,000,000,000 4,000,000,000 +7,000,000,000 6,000,000,000 8,000,000,000 -7,000,000,000 -11,000,000,000 -19,000,000,000 +9,000,000,000 15,000,000,000 12,000,000,000 -14,000,000,000 +11,000,000,000 13,000,000,000 +14,000,000,000 +17,000,000,000 16,000,000,000 18,000,000,000 -17,000,000,000 +19,000,000,000 - - + + - - - + - + + + - + diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 8e871952c2..226259a53a 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -47,11 +47,11 @@ test_that("show.legend handles named vectors", { }) test_that("axis_label_overlap_priority always returns the correct number of elements", { - expect_identical(axis_label_overlap_priority(0), numeric(0)) - expect_setequal(axis_label_overlap_priority(1), seq_len(1)) - expect_setequal(axis_label_overlap_priority(5), seq_len(5)) - expect_setequal(axis_label_overlap_priority(10), seq_len(10)) - expect_setequal(axis_label_overlap_priority(100), seq_len(100)) + expect_identical(axis_label_priority(0), numeric(0)) + expect_setequal(axis_label_priority(1), seq_len(1)) + expect_setequal(axis_label_priority(5), seq_len(5)) + expect_setequal(axis_label_priority(10), seq_len(10)) + expect_setequal(axis_label_priority(100), seq_len(100)) }) test_that("axis_label_element_overrides errors when angles are outside the range [0, 90]", { From 2c65cbbecd58b09532a1a9b63ddcb42a4d28bc07 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Wed, 19 Jun 2019 15:44:54 -0400 Subject: [PATCH 05/11] add support for axis dodging into multiple rows/cols --- R/guides-axis.r | 83 +++++++---- .../figs/guides/axis-guides-check-overlap.svg | 64 ++++---- ...is-guides-texted-dodged-into-rows-cols.svg | 140 ++++++++++++++++++ tests/testthat/test-guides.R | 6 + 4 files changed, 233 insertions(+), 60 deletions(-) create mode 100644 tests/figs/guides/axis-guides-texted-dodged-into-rows-cols.svg diff --git a/R/guides-axis.r b/R/guides-axis.r index 89b94aff9d..89c26cef3e 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -10,11 +10,14 @@ #' @param angle The angle at which the text should be rotated (between #' -90 and 90), or `NULL` if the angle should be obtained from the #' `theme`. +#' @param n_dodge The number of rows (for vertical axes) or columns (for +#' horizontal axes) that should be used to render the labels. This is +#' useful for displaying labels that would otherwise overlap. #' #' @noRd #' draw_axis <- function(break_positions, break_labels, axis_position, theme, - check.overlap = FALSE, angle = NULL) { + check.overlap = FALSE, angle = NULL, n_dodge = 1) { axis_position <- match.arg(axis_position, c("top", "bottom", "right", "left")) aesthetic <- if (axis_position %in% c("top", "bottom")) "x" else "y" @@ -61,8 +64,6 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, # conditionally set the gtable ordering labels_first_gtable <- axis_position %in% c("left", "top") # refers to position in gtable - table_order <- if (labels_first_gtable) c("labels", "ticks") else c("ticks", "labels") - # set common parameters n_breaks <- length(break_positions) opposite_positions <- c("top" = "bottom", "bottom" = "top", "right" = "left", "left" = "right") @@ -85,28 +86,20 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, ) } - # break_labels can be a list() of language objects - if (is.list(break_labels)) { - if (any(vapply(break_labels, is.language, logical(1)))) { - break_labels <- do.call(expression, break_labels) - } else { - break_labels <- unlist(break_labels) - } - } - - if (check.overlap) { - priority <- axis_label_priority(n_breaks) - break_labels <- break_labels[priority] - break_positions <- break_positions[priority] - } - - labels_grob <- exec( - element_grob, label_element, - !!position_dim := unit(break_positions, "native"), - !!label_margin_name := TRUE, - label = break_labels, - check.overlap = check.overlap - ) + # calculate multiple rows/columns of labels (which is usually 1) + dodge_pos <- rep(seq_len(n_dodge), length.out = n_breaks) + dodge_indices <- split(seq_len(n_breaks), dodge_pos) + + label_grobs <- lapply(dodge_indices, function(indices) { + draw_axis_labels( + break_positions = break_positions[indices], + break_labels = break_labels[indices], + label_element = label_element, + position_dim = position_dim, + label_margin_name = label_margin_name, + check.overlap = check.overlap + ) + }) ticks_grob <- exec( element_grob, tick_element, @@ -119,14 +112,21 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, ) # create gtable - table_order_int <- match(table_order, c("labels", "ticks")) non_position_sizes <- paste0(non_position_size, "s") + label_dims <- do.call(unit.c, lapply(label_grobs, measure_labels)) + grobs <- c(list(ticks_grob), label_grobs) + grob_dims <- unit.c(tick_length, label_dims) + + if (labels_first_gtable) { + grobs <- rev(grobs) + grob_dims <- rev(grob_dims) + } gt <- exec( gtable_element, name = "axis", - grobs = list(labels_grob, ticks_grob)[table_order_int], - !!non_position_sizes := unit.c(measure_labels(labels_grob), tick_length)[table_order_int], + grobs = grobs, + !!non_position_sizes := grob_dims, !!position_size := unit(1, "npc") ) @@ -146,6 +146,33 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, ) } +draw_axis_labels <- function(break_positions, break_labels, label_element, + position_dim, label_margin_name, check.overlap) { + + # break_labels can be a list() of language objects + if (is.list(break_labels)) { + if (any(vapply(break_labels, is.language, logical(1)))) { + break_labels <- do.call(expression, break_labels) + } else { + break_labels <- unlist(break_labels) + } + } + + if (check.overlap) { + priority <- axis_label_priority(length(break_positions)) + break_labels <- break_labels[priority] + break_positions <- break_positions[priority] + } + + labels_grob <- exec( + element_grob, label_element, + !!position_dim := unit(break_positions, "native"), + !!label_margin_name := TRUE, + label = break_labels, + check.overlap = check.overlap + ) +} + #' Deterine the label priority for a given number of labels #' #' @param n The number of labels diff --git a/tests/figs/guides/axis-guides-check-overlap.svg b/tests/figs/guides/axis-guides-check-overlap.svg index 35615f5d63..dc9faa4c64 100644 --- a/tests/figs/guides/axis-guides-check-overlap.svg +++ b/tests/figs/guides/axis-guides-check-overlap.svg @@ -64,46 +64,46 @@ 12,000,000,000 17,000,000,000 - - - - + - + + - - + + - + + + - - - - + - + + - - + + - + + + 1,000,000,000 20,000,000,000 10,000,000,000 @@ -126,25 +126,25 @@ 19,000,000,000 - - - - + - + + - - + + - + + + 1,000,000,000 20,000,000,000 10,000,000,000 @@ -176,23 +176,23 @@ 18,000,000,000 19,000,000,000 - - - - + - + + - - + + - + + + diff --git a/tests/figs/guides/axis-guides-texted-dodged-into-rows-cols.svg b/tests/figs/guides/axis-guides-texted-dodged-into-rows-cols.svg new file mode 100644 index 0000000000..224723b0c0 --- /dev/null +++ b/tests/figs/guides/axis-guides-texted-dodged-into-rows-cols.svg @@ -0,0 +1,140 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +2,000,000,000 +4,000,000,000 +6,000,000,000 +8,000,000,000 +10,000,000,000 +1,000,000,000 +3,000,000,000 +5,000,000,000 +7,000,000,000 +9,000,000,000 + + + + + + + + + + + + + + + + + + + + + +1,000,000,000 +3,000,000,000 +5,000,000,000 +7,000,000,000 +9,000,000,000 +2,000,000,000 +4,000,000,000 +6,000,000,000 +8,000,000,000 +10,000,000,000 + + + + + + + + + + + +1,000,000,000 +3,000,000,000 +5,000,000,000 +7,000,000,000 +9,000,000,000 +2,000,000,000 +4,000,000,000 +6,000,000,000 +8,000,000,000 +10,000,000,000 + +2,000,000,000 +4,000,000,000 +6,000,000,000 +8,000,000,000 +10,000,000,000 +1,000,000,000 +3,000,000,000 +5,000,000,000 +7,000,000,000 +9,000,000,000 + + + + + + + + + + + diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 226259a53a..136b3b4f38 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -123,6 +123,12 @@ test_that("axis guides are drawn correctly", { "axis guides, vertical negative rotation", function() test_draw_axis(10, labels = function(b) comma(b * 1e3), angle = -90) ) + + # dodged text + expect_doppelganger( + "axis guides, texted dodged into rows/cols", + function() test_draw_axis(10, labels = function(b) comma(b * 1e9), n_dodge = 2) + ) }) test_that("axis guides are drawn correctly in plots", { From c27cb3c349e07e6e03e2210a919cd386d4d2c160 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 20 Jun 2019 09:37:27 -0400 Subject: [PATCH 06/11] update documentation for new axis options --- R/guides-axis.r | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/guides-axis.r b/R/guides-axis.r index 89c26cef3e..99c56ecb75 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -7,9 +7,9 @@ #' @param theme A complete [theme()] object #' @param check.overlap silently remove overlapping labels, #' (recursively) prioritizing the first, last, and middle labels. -#' @param angle The angle at which the text should be rotated (between -#' -90 and 90), or `NULL` if the angle should be obtained from the -#' `theme`. +#' @param angle Compared to setting the angle in [theme()] / [element_text()], +#' this also uses some heuristics to automatically pick the `hjust` and `vjust` that +#' you probably want. #' @param n_dodge The number of rows (for vertical axes) or columns (for #' horizontal axes) that should be used to render the labels. This is #' useful for displaying labels that would otherwise overlap. @@ -173,7 +173,7 @@ draw_axis_labels <- function(break_positions, break_labels, label_element, ) } -#' Deterine the label priority for a given number of labels +#' Determine the label priority for a given number of labels #' #' @param n The number of labels #' From cde092bed25baca4303271b8f5ac38264cfdfbbd Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 20 Jun 2019 09:41:08 -0400 Subject: [PATCH 07/11] move break label sanitizing back to draw_axis() --- R/guides-axis.r | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/R/guides-axis.r b/R/guides-axis.r index 99c56ecb75..61115f9c76 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -86,6 +86,15 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, ) } + # break_labels can be a list() of language objects + if (is.list(break_labels)) { + if (any(vapply(break_labels, is.language, logical(1)))) { + break_labels <- do.call(expression, break_labels) + } else { + break_labels <- unlist(break_labels) + } + } + # calculate multiple rows/columns of labels (which is usually 1) dodge_pos <- rep(seq_len(n_dodge), length.out = n_breaks) dodge_indices <- split(seq_len(n_breaks), dodge_pos) @@ -149,15 +158,6 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, draw_axis_labels <- function(break_positions, break_labels, label_element, position_dim, label_margin_name, check.overlap) { - # break_labels can be a list() of language objects - if (is.list(break_labels)) { - if (any(vapply(break_labels, is.language, logical(1)))) { - break_labels <- do.call(expression, break_labels) - } else { - break_labels <- unlist(break_labels) - } - } - if (check.overlap) { priority <- axis_label_priority(length(break_positions)) break_labels <- break_labels[priority] From fcddb3aee72fddbaa93b5817ed33c2e5a5c8bd82 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 20 Jun 2019 12:26:00 -0400 Subject: [PATCH 08/11] add optional end label alignment to draw_axis() --- R/guides-axis.r | 60 +++++++++++++---- .../axis-guides-ends-aligned-horizontal.svg | 66 +++++++++++++++++++ ...-guides-ends-aligned-vertical-negative.svg | 66 +++++++++++++++++++ ...-guides-ends-aligned-vertical-positive.svg | 66 +++++++++++++++++++ ...xis-guides-text-dodged-into-rows-cols.svg} | 0 tests/testthat/test-guides.R | 57 +++++++++++++++- 6 files changed, 301 insertions(+), 14 deletions(-) create mode 100644 tests/figs/guides/axis-guides-ends-aligned-horizontal.svg create mode 100644 tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg create mode 100644 tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg rename tests/figs/guides/{axis-guides-texted-dodged-into-rows-cols.svg => axis-guides-text-dodged-into-rows-cols.svg} (100%) diff --git a/R/guides-axis.r b/R/guides-axis.r index 61115f9c76..5a3c9b224b 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -13,11 +13,14 @@ #' @param n_dodge The number of rows (for vertical axes) or columns (for #' horizontal axes) that should be used to render the labels. This is #' useful for displaying labels that would otherwise overlap. +#' @param align_ends Ensure end labels are within the bounds of the axis +#' space. #' #' @noRd #' draw_axis <- function(break_positions, break_labels, axis_position, theme, - check.overlap = FALSE, angle = NULL, n_dodge = 1) { + check.overlap = FALSE, angle = NULL, n_dodge = 1, + align_ends = FALSE) { axis_position <- match.arg(axis_position, c("top", "bottom", "right", "left")) aesthetic <- if (axis_position %in% c("top", "bottom")) "x" else "y" @@ -48,10 +51,9 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, non_position_dim <- if (is_vertical) "x" else "y" position_size <- if (is_vertical) "height" else "width" non_position_size <- if (is_vertical) "width" else "height" - label_margin_name <- if (is_vertical) "margin_x" else "margin_y" gtable_element <- if (is_vertical) gtable_row else gtable_col measure_gtable <- if (is_vertical) gtable_width else gtable_height - measure_labels <- if (is_vertical) grobWidth else grobHeight + measure_labels_non_pos <- if (is_vertical) grobWidth else grobHeight # conditionally set parameters that depend on which side of the panel # the axis is on @@ -104,9 +106,9 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, break_positions = break_positions[indices], break_labels = break_labels[indices], label_element = label_element, - position_dim = position_dim, - label_margin_name = label_margin_name, - check.overlap = check.overlap + is_vertical = is_vertical, + check.overlap = check.overlap, + align_ends = align_ends ) }) @@ -122,7 +124,7 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, # create gtable non_position_sizes <- paste0(non_position_size, "s") - label_dims <- do.call(unit.c, lapply(label_grobs, measure_labels)) + label_dims <- do.call(unit.c, lapply(label_grobs, measure_labels_non_pos)) grobs <- c(list(ticks_grob), label_grobs) grob_dims <- unit.c(tick_length, label_dims) @@ -155,18 +157,54 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, ) } -draw_axis_labels <- function(break_positions, break_labels, label_element, - position_dim, label_margin_name, check.overlap) { +draw_axis_labels <- function(break_positions, break_labels, label_element, is_vertical, + check.overlap = FALSE, align_ends = FALSE) { + + position_dim <- if (is_vertical) "y" else "x" + label_margin_name <- if (is_vertical) "margin_x" else "margin_y" + measure_labels_pos <- if (is_vertical) grobHeight else grobWidth + + n_breaks <- length(break_positions) + break_positions <- unit(break_positions, "native") + + if (align_ends) { + + # getting the alignment code to work properly for text that isn't parallel + # to the axis is a lot of effort for little gain + if (is_vertical && abs(label_element$angle) != 90) { + stop("Cannot align end labels for vertical axis when `angle` is not 90 or -90", call. = FALSE) + } + if (!is_vertical && label_element$angle != 0) { + stop("Cannot align end labels for horizontal axis when `angle` is not 0", call. = FALSE) + } + + first_label <- exec(element_grob, label_element, label = break_labels[1]) + last_label <- exec(element_grob, label_element, label = break_labels[n_breaks]) + + first_label_dim <- measure_labels_pos(first_label) + last_label_dim <- measure_labels_pos(last_label) + just <- label_element$hjust + + break_positions[1] <- max( + break_positions[1], + unit(0, "npc") + first_label_dim * just + ) + + break_positions[n_breaks] <- min( + break_positions[n_breaks], + unit(1, "npc") - last_label_dim * (1 - just) + ) + } if (check.overlap) { - priority <- axis_label_priority(length(break_positions)) + priority <- axis_label_priority(n_breaks) break_labels <- break_labels[priority] break_positions <- break_positions[priority] } labels_grob <- exec( element_grob, label_element, - !!position_dim := unit(break_positions, "native"), + !!position_dim := break_positions, !!label_margin_name := TRUE, label = break_labels, check.overlap = check.overlap diff --git a/tests/figs/guides/axis-guides-ends-aligned-horizontal.svg b/tests/figs/guides/axis-guides-ends-aligned-horizontal.svg new file mode 100644 index 0000000000..7e7769b55a --- /dev/null +++ b/tests/figs/guides/axis-guides-ends-aligned-horizontal.svg @@ -0,0 +1,66 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000,000,000 +2,000,000,000 + + + + + +1,000,000,000 +2,000,000,000 + diff --git a/tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg b/tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg new file mode 100644 index 0000000000..98d1e35823 --- /dev/null +++ b/tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg @@ -0,0 +1,66 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000,000,000 +2,000,000,000 + + + + + +1,000,000,000 +2,000,000,000 + diff --git a/tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg b/tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg new file mode 100644 index 0000000000..adc456e04a --- /dev/null +++ b/tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg @@ -0,0 +1,66 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +1,000,000,000 +2,000,000,000 + + + + + +1,000,000,000 +2,000,000,000 + diff --git a/tests/figs/guides/axis-guides-texted-dodged-into-rows-cols.svg b/tests/figs/guides/axis-guides-text-dodged-into-rows-cols.svg similarity index 100% rename from tests/figs/guides/axis-guides-texted-dodged-into-rows-cols.svg rename to tests/figs/guides/axis-guides-text-dodged-into-rows-cols.svg diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index 136b3b4f38..ef968586c8 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -60,18 +60,29 @@ test_that("axis_label_element_overrides errors when angles are outside the range expect_error(axis_label_element_overrides("bottom", -91), "`angle` must") }) +test_that("label end alignment errors when alignment is not possible", { + expect_error( + draw_axis(1, "1", "left", theme_test(), angle = 0, align_ends = TRUE), + "Cannot align end labels" + ) + expect_error( + draw_axis(1, "1", "bottom", theme_test(), angle = 90, align_ends = TRUE), + "Cannot align end labels" + ) +}) + # Visual tests ------------------------------------------------------------ test_that("axis guides are drawn correctly", { theme_test_axis <- theme_test() + theme(axis.line = element_line(size = 0.5)) test_draw_axis <- function(n_breaks = 3, + break_positions = seq_len(n_breaks) / (n_breaks + 1), labels = as.character, positions = c("top", "right", "bottom", "left"), theme = theme_test_axis, ...) { - break_positions <- seq_len(n_breaks) / (n_breaks + 1) - break_labels <- labels(seq_len(n_breaks)) + break_labels <- labels(seq_along(break_positions)) # create the axes axes <- lapply(positions, function(position) { @@ -126,9 +137,49 @@ test_that("axis guides are drawn correctly", { # dodged text expect_doppelganger( - "axis guides, texted dodged into rows/cols", + "axis guides, text dodged into rows/cols", function() test_draw_axis(10, labels = function(b) comma(b * 1e9), n_dodge = 2) ) + + # end alignment + expect_doppelganger( + "axis guides, ends aligned (horizontal)", + function() { + test_draw_axis( + break_positions = c(0.01, 0.99), + labels = function(b) comma(b * 1e9), + positions = c("top", "bottom"), + angle = 0, + align_ends = TRUE + ) + } + ) + + expect_doppelganger( + "axis guides, ends aligned (vertical positive)", + function() { + test_draw_axis( + break_positions = c(0.01, 0.99), + labels = function(b) comma(b * 1e9), + positions = c("left", "right"), + angle = 90, + align_ends = TRUE + ) + } + ) + + expect_doppelganger( + "axis guides, ends aligned (vertical negative)", + function() { + test_draw_axis( + break_positions = c(0.01, 0.99), + labels = function(b) comma(b * 1e9), + positions = c("left", "right"), + angle = -90, + align_ends = TRUE + ) + } + ) }) test_that("axis guides are drawn correctly in plots", { From aab843d9876e1c8f515569a77a3b0d93ab815bfb Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 20 Jun 2019 12:26:14 -0400 Subject: [PATCH 09/11] clean orphaned visual test case --- tests/figs/themes/ticks_length.svg | 74 ------------------------------ 1 file changed, 74 deletions(-) delete mode 100644 tests/figs/themes/ticks_length.svg diff --git a/tests/figs/themes/ticks_length.svg b/tests/figs/themes/ticks_length.svg deleted file mode 100644 index 54c82ae3b1..0000000000 --- a/tests/figs/themes/ticks_length.svg +++ /dev/null @@ -1,74 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -2.5 -5.0 -7.5 -10.0 - - - - -2.5 -5.0 -7.5 -10.0 - - - - - - - - -2.5 -5.0 -7.5 -10.0 - - - - -2.5 -5.0 -7.5 -10.0 -1:10 -1:10 -1:10 -1:10 - From bee5b2aa1b145d2e43cc6abbc089c1b58094d90e Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 20 Jun 2019 14:11:14 -0400 Subject: [PATCH 10/11] apply alignment to all labels (not just first and last). This fixes the Travis failure on R 3.2, where `unit.[<-` appears to have different behaviour --- R/guides-axis.r | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/R/guides-axis.r b/R/guides-axis.r index 5a3c9b224b..3a354cd6a6 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -178,21 +178,24 @@ draw_axis_labels <- function(break_positions, break_labels, label_element, is_ve stop("Cannot align end labels for horizontal axis when `angle` is not 0", call. = FALSE) } - first_label <- exec(element_grob, label_element, label = break_labels[1]) - last_label <- exec(element_grob, label_element, label = break_labels[n_breaks]) + # get the (to be evaluated at draw time) size of each label in the position dimension + label_dims <- do.call( + unit.c, + lapply(break_labels, function(label) { + measure_labels_pos(exec(element_grob, label_element, label = label)) + }) + ) - first_label_dim <- measure_labels_pos(first_label) - last_label_dim <- measure_labels_pos(last_label) just <- label_element$hjust - break_positions[1] <- max( - break_positions[1], - unit(0, "npc") + first_label_dim * just + break_positions <- unit.pmax( + break_positions, + unit(0, "npc") + label_dims * just ) - break_positions[n_breaks] <- min( - break_positions[n_breaks], - unit(1, "npc") - last_label_dim * (1 - just) + break_positions <- unit.pmin( + break_positions, + unit(1, "npc") - label_dims * (1 - just) ) } From d6cde07de23a230e08a717d60e006655844691f1 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Fri, 21 Jun 2019 10:04:01 -0400 Subject: [PATCH 11/11] remove align_ends option --- R/guides-axis.r | 43 +----------- .../axis-guides-ends-aligned-horizontal.svg | 66 ------------------- ...-guides-ends-aligned-vertical-negative.svg | 66 ------------------- ...-guides-ends-aligned-vertical-positive.svg | 66 ------------------- tests/testthat/test-guides.R | 51 -------------- 5 files changed, 3 insertions(+), 289 deletions(-) delete mode 100644 tests/figs/guides/axis-guides-ends-aligned-horizontal.svg delete mode 100644 tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg delete mode 100644 tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg diff --git a/R/guides-axis.r b/R/guides-axis.r index 3a354cd6a6..3b13fcea06 100644 --- a/R/guides-axis.r +++ b/R/guides-axis.r @@ -13,14 +13,11 @@ #' @param n_dodge The number of rows (for vertical axes) or columns (for #' horizontal axes) that should be used to render the labels. This is #' useful for displaying labels that would otherwise overlap. -#' @param align_ends Ensure end labels are within the bounds of the axis -#' space. #' #' @noRd #' draw_axis <- function(break_positions, break_labels, axis_position, theme, - check.overlap = FALSE, angle = NULL, n_dodge = 1, - align_ends = FALSE) { + check.overlap = FALSE, angle = NULL, n_dodge = 1) { axis_position <- match.arg(axis_position, c("top", "bottom", "right", "left")) aesthetic <- if (axis_position %in% c("top", "bottom")) "x" else "y" @@ -107,8 +104,7 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, break_labels = break_labels[indices], label_element = label_element, is_vertical = is_vertical, - check.overlap = check.overlap, - align_ends = align_ends + check.overlap = check.overlap ) }) @@ -158,47 +154,14 @@ draw_axis <- function(break_positions, break_labels, axis_position, theme, } draw_axis_labels <- function(break_positions, break_labels, label_element, is_vertical, - check.overlap = FALSE, align_ends = FALSE) { + check.overlap = FALSE) { position_dim <- if (is_vertical) "y" else "x" label_margin_name <- if (is_vertical) "margin_x" else "margin_y" - measure_labels_pos <- if (is_vertical) grobHeight else grobWidth n_breaks <- length(break_positions) break_positions <- unit(break_positions, "native") - if (align_ends) { - - # getting the alignment code to work properly for text that isn't parallel - # to the axis is a lot of effort for little gain - if (is_vertical && abs(label_element$angle) != 90) { - stop("Cannot align end labels for vertical axis when `angle` is not 90 or -90", call. = FALSE) - } - if (!is_vertical && label_element$angle != 0) { - stop("Cannot align end labels for horizontal axis when `angle` is not 0", call. = FALSE) - } - - # get the (to be evaluated at draw time) size of each label in the position dimension - label_dims <- do.call( - unit.c, - lapply(break_labels, function(label) { - measure_labels_pos(exec(element_grob, label_element, label = label)) - }) - ) - - just <- label_element$hjust - - break_positions <- unit.pmax( - break_positions, - unit(0, "npc") + label_dims * just - ) - - break_positions <- unit.pmin( - break_positions, - unit(1, "npc") - label_dims * (1 - just) - ) - } - if (check.overlap) { priority <- axis_label_priority(n_breaks) break_labels <- break_labels[priority] diff --git a/tests/figs/guides/axis-guides-ends-aligned-horizontal.svg b/tests/figs/guides/axis-guides-ends-aligned-horizontal.svg deleted file mode 100644 index 7e7769b55a..0000000000 --- a/tests/figs/guides/axis-guides-ends-aligned-horizontal.svg +++ /dev/null @@ -1,66 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -1,000,000,000 -2,000,000,000 - - - - - -1,000,000,000 -2,000,000,000 - diff --git a/tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg b/tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg deleted file mode 100644 index 98d1e35823..0000000000 --- a/tests/figs/guides/axis-guides-ends-aligned-vertical-negative.svg +++ /dev/null @@ -1,66 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -1,000,000,000 -2,000,000,000 - - - - - -1,000,000,000 -2,000,000,000 - diff --git a/tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg b/tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg deleted file mode 100644 index adc456e04a..0000000000 --- a/tests/figs/guides/axis-guides-ends-aligned-vertical-positive.svg +++ /dev/null @@ -1,66 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -1,000,000,000 -2,000,000,000 - - - - - -1,000,000,000 -2,000,000,000 - diff --git a/tests/testthat/test-guides.R b/tests/testthat/test-guides.R index ef968586c8..2bd7d0b508 100644 --- a/tests/testthat/test-guides.R +++ b/tests/testthat/test-guides.R @@ -60,17 +60,6 @@ test_that("axis_label_element_overrides errors when angles are outside the range expect_error(axis_label_element_overrides("bottom", -91), "`angle` must") }) -test_that("label end alignment errors when alignment is not possible", { - expect_error( - draw_axis(1, "1", "left", theme_test(), angle = 0, align_ends = TRUE), - "Cannot align end labels" - ) - expect_error( - draw_axis(1, "1", "bottom", theme_test(), angle = 90, align_ends = TRUE), - "Cannot align end labels" - ) -}) - # Visual tests ------------------------------------------------------------ test_that("axis guides are drawn correctly", { @@ -140,46 +129,6 @@ test_that("axis guides are drawn correctly", { "axis guides, text dodged into rows/cols", function() test_draw_axis(10, labels = function(b) comma(b * 1e9), n_dodge = 2) ) - - # end alignment - expect_doppelganger( - "axis guides, ends aligned (horizontal)", - function() { - test_draw_axis( - break_positions = c(0.01, 0.99), - labels = function(b) comma(b * 1e9), - positions = c("top", "bottom"), - angle = 0, - align_ends = TRUE - ) - } - ) - - expect_doppelganger( - "axis guides, ends aligned (vertical positive)", - function() { - test_draw_axis( - break_positions = c(0.01, 0.99), - labels = function(b) comma(b * 1e9), - positions = c("left", "right"), - angle = 90, - align_ends = TRUE - ) - } - ) - - expect_doppelganger( - "axis guides, ends aligned (vertical negative)", - function() { - test_draw_axis( - break_positions = c(0.01, 0.99), - labels = function(b) comma(b * 1e9), - positions = c("left", "right"), - angle = -90, - align_ends = TRUE - ) - } - ) }) test_that("axis guides are drawn correctly in plots", {