From 06f5234ce1f9276c7902a625301f0d5c69487ae9 Mon Sep 17 00:00:00 2001 From: Dana Seidel Date: Thu, 2 Aug 2018 12:04:39 -0700 Subject: [PATCH 1/2] Simplify sec.axis for proper transformed breaks --- NEWS.md | 3 +++ R/axis-secondary.R | 18 +++++----------- tests/testthat/test-sec-axis.R | 39 ++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/NEWS.md b/NEWS.md index 85bb67443f..eb03b11aa4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # ggplot2 3.0.0.9000 +* `sec_axis()` and `dup_axis()` now return appropriate breaks for the secondary + axis when applied to log transformed scales (@dpseidel, #2729). + * `stat_contour()`, `stat_density2d()`, `stat_bin2d()`, `stat_binhex()` now calculate normalized statistics including `nlevel`, `ndensity`, and `ncount`. Also, `stat_density()` now includes the calculated statistic diff --git a/R/axis-secondary.R b/R/axis-secondary.R index 5d216ce635..3c7ee2964d 100644 --- a/R/axis-secondary.R +++ b/R/axis-secondary.R @@ -109,7 +109,6 @@ AxisSecondary <- ggproto("AxisSecondary", NULL, f_eval(self$trans, range) }, - break_info = function(self, range, scale) { if (self$empty()) return() @@ -125,29 +124,22 @@ AxisSecondary <- ggproto("AxisSecondary", NULL, stop("transformation for secondary axes must be monotonic") # Get break info for the secondary axis - new_range <- range(full_range, na.rm = TRUE) - temp_scale <- self$create_scale(new_range) - range_info <- temp_scale$break_info() - - # Map the break values back to their correct position on the primary scale - old_val <- lapply(range_info$major_source, function(x) which.min(abs(full_range - x))) - old_val <- old_range[unlist(old_val)] - old_val_trans <- scale$trans$transform(old_val) - range_info$major[] <- round(rescale(scale$map(old_val_trans, range(old_val_trans)), from = range), digits = 3) - + new_range <- range(scale$transform(full_range), na.rm = TRUE) + sec_scale <- self$create_scale(new_range, scale) + range_info <- sec_scale$break_info() names(range_info) <- paste0("sec.", names(range_info)) range_info }, # Temporary scale for the purpose of calling break_info() - create_scale = function(self, range) { + create_scale = function(self, range, primary) { scale <- ggproto(NULL, ScaleContinuousPosition, name = self$name, breaks = self$breaks, labels = self$labels, limits = range, expand = c(0, 0), - trans = identity_trans() + trans = primary$trans ) scale$train(range) scale diff --git a/tests/testthat/test-sec-axis.R b/tests/testthat/test-sec-axis.R index 2f69eeda90..d356cdd46b 100644 --- a/tests/testthat/test-sec-axis.R +++ b/tests/testthat/test-sec-axis.R @@ -18,6 +18,45 @@ test_that("dup_axis() works", { expect_equal(breaks$major_source, breaks$sec.major_source) }) +test_that("sec_axis() breaks work for log-transformed scales", { + df <- data.frame( + x = c("A", "B", "C"), + y = c(10, 100, 1000) + ) + + #dup_axis() + p <- ggplot(data = df, aes(x, y)) + + geom_point() + + scale_y_log10(sec.axis = dup_axis()) + + scale <- layer_scales(p)$y + breaks <- scale$break_info() + + expect_equal(breaks$major_source, breaks$sec.major_source) + + #sec_axis() with transform + p <- ggplot(data = df, aes(x, y)) + + geom_point() + + scale_y_log10(sec.axis = sec_axis(~.*100)) + + scale <- layer_scales(p)$y + breaks <- scale$break_info() + + expect_equal(breaks$major_source, breaks$sec.major_source - 2) + + #sec_axis() with transform and breaks + custom_breaks <- c(10, 20, 40, 200, 400, 800) + p <- ggplot(data = df, aes(x, y)) + + geom_point() + + scale_y_log10(breaks = custom_breaks, sec.axis = sec_axis(~.*100)) + + scale <- layer_scales(p)$y + breaks <- scale$break_info() + + expect_equal(breaks$major_source, log(custom_breaks, base = 10)) + expect_equal(log_breaks()(df$y)*100, 10^(breaks$sec.major_source)) +}) + test_that("custom breaks work", { custom_breaks <- c(0.01, 0.1, 1, 10, 100) p <- ggplot(foo, aes(x, y)) + From ef67905938c4e8d490bedce02cd06f9f7021e893 Mon Sep 17 00:00:00 2001 From: Dana Seidel Date: Mon, 6 Aug 2018 14:01:05 -0700 Subject: [PATCH 2/2] Tidy style tests --- tests/testthat/test-sec-axis.R | 36 ++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/tests/testthat/test-sec-axis.R b/tests/testthat/test-sec-axis.R index d356cdd46b..619d517210 100644 --- a/tests/testthat/test-sec-axis.R +++ b/tests/testthat/test-sec-axis.R @@ -3,14 +3,16 @@ context("sec-axis") x <- exp(seq(log(0.001), log(1000), length.out = 100)) foo <- data.frame( x = x, - y = x/(1+x) + y = x / (1 + x) ) test_that("dup_axis() works", { p <- ggplot(foo, aes(x, y)) + geom_point() + - scale_x_continuous(name = "Unit A", - sec.axis = dup_axis()) + scale_x_continuous( + name = "Unit A", + sec.axis = dup_axis() + ) scale <- layer_scales(p)$x expect_equal(scale$sec_name(), scale$name) breaks <- scale$break_info() @@ -24,7 +26,7 @@ test_that("sec_axis() breaks work for log-transformed scales", { y = c(10, 100, 1000) ) - #dup_axis() + # dup_axis() p <- ggplot(data = df, aes(x, y)) + geom_point() + scale_y_log10(sec.axis = dup_axis()) @@ -34,27 +36,27 @@ test_that("sec_axis() breaks work for log-transformed scales", { expect_equal(breaks$major_source, breaks$sec.major_source) - #sec_axis() with transform + # sec_axis() with transform p <- ggplot(data = df, aes(x, y)) + geom_point() + - scale_y_log10(sec.axis = sec_axis(~.*100)) + scale_y_log10(sec.axis = sec_axis(~. * 100)) scale <- layer_scales(p)$y breaks <- scale$break_info() expect_equal(breaks$major_source, breaks$sec.major_source - 2) - #sec_axis() with transform and breaks + # sec_axis() with transform and breaks custom_breaks <- c(10, 20, 40, 200, 400, 800) p <- ggplot(data = df, aes(x, y)) + geom_point() + - scale_y_log10(breaks = custom_breaks, sec.axis = sec_axis(~.*100)) + scale_y_log10(breaks = custom_breaks, sec.axis = sec_axis(~. * 100)) scale <- layer_scales(p)$y breaks <- scale$break_info() expect_equal(breaks$major_source, log(custom_breaks, base = 10)) - expect_equal(log_breaks()(df$y)*100, 10^(breaks$sec.major_source)) + expect_equal(log_breaks()(df$y) * 100, 10^(breaks$sec.major_source)) }) test_that("custom breaks work", { @@ -64,7 +66,7 @@ test_that("custom breaks work", { scale_x_continuous( name = "Unit A", sec.axis = sec_axis( - trans = y~., + trans = y ~ ., breaks = custom_breaks ) ) @@ -78,10 +80,14 @@ test_that("sec axis works with skewed transform", { "sec_axis, skewed transform", ggplot(foo, aes(x, y)) + geom_point() + - scale_x_continuous(name = "Unit A", trans = 'log', - breaks = c(0.001, 0.01, 0.1, 1, 10, 100, 1000), - sec.axis = sec_axis(~. * 100, name = "Unit B", - labels = derive(), - breaks = derive())) + scale_x_continuous( + name = "Unit A", trans = "log", + breaks = c(0.001, 0.01, 0.1, 1, 10, 100, 1000), + sec.axis = sec_axis(~. * 100, + name = "Unit B", + labels = derive(), + breaks = derive() + ) + ) ) })