Simplify sec.axis for proper transformed breaks

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).
+
 *   `sec_axis()` now works as expected when used in combination with tidy eval
     (@dpseidel, #2788).
 

R/axis-secondary.R

@@ -109,7 +109,6 @@ AxisSecondary <- ggproto("AxisSecondary", NULL,
     rlang::eval_tidy(rlang::f_rhs(self$trans), data = 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

tests/testthat/test-sec-axis.R

@@ -3,29 +3,70 @@ 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()
   expect_equal(breaks$minor, breaks$sec.minor)
   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)) +
     geom_point() +
     scale_x_continuous(
       name = "Unit A",
       sec.axis = sec_axis(
-        trans = y~.,
+        trans = y ~ .,
         breaks = custom_breaks
       )
     )
@@ -39,11 +80,15 @@ 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()
+        )
+      )
   )
 })