From d407103900c9cdbb67cdaeaeaa702d58c8dc2436 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Thu, 20 Nov 2025 20:09:35 -0800 Subject: [PATCH 1/7] store + retrieve axis labs, then re-order datapoints --- R/tinyplot.R | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/R/tinyplot.R b/R/tinyplot.R index 7785bcdc..8a47f725 100644 --- a/R/tinyplot.R +++ b/R/tinyplot.R @@ -913,7 +913,6 @@ tinyplot.default = function( # ## legends ----- # - # browser() # legend labels ncolors = length(col) @@ -1186,6 +1185,46 @@ tinyplot.default = function( ## split and draw datapoints ----- # + # layering gotcha for ordered factors along the x-axis (e.g., when adding + # lines or ribbons on top of errorbars) + if (!add) { + # keep track of the x(y)labs from the original (1st) layer + assign("xlabs", xlabs, envir = get(".tinyplot_env", envir = parent.env(environment()))) + assign("ylabs", ylabs, envir = get(".tinyplot_env", envir = parent.env(environment()))) + } else { + # check whether we need to adjust any added layers + # aside: we need some extra accounting for flipped plots (x -> y) + if (!flip) { + labs_orig = "xlabs" + labs_new = xlabs + dp_var = "x" + } else { + labs_orig = "ylabs" + labs_new = ylabs + dp_var = "y" + } + # retrieve the relevant axis labs from the original layer (and we only care + # if it's not null) + labs_orig = get(labs_orig, envir = get(".tinyplot_env", envir = parent.env(environment()))) + if (!is.null(names(labs_orig))) { + if (is.factor(datapoints[[dp_var]])) { + # case 1: relevel a factor (e.g., ribbon added to errorbars) + datapoints[[dp_var]] = tryCatch( + factor(datapoints[[dp_var]], levels = names(labs_orig)), + error = function(e) { + datapoints[[dp_var]] + } + ) + } else if (!is.null(names(labs_new))) { + # case 2: match implicit integer -> label mapping (e.g., lines added to errorbars) + if (setequal(names(labs_new), names(labs_orig))) { + datapoints[[dp_var]] = labs_orig[names(labs_new)[datapoints[[dp_var]]]] + datapoints = datapoints[order(datapoints[[dp_var]]), ] + } + } + } + } + # Finally, we can draw all of the plot elements (points, lines, etc.) # We'll do this via a nested loops: # 1) Outer loop over facets @@ -1397,7 +1436,6 @@ tinyplot.formula = function( ## placeholder for legend title legend_args = list(x = NULL) - # browser() ## turn facet into a formula if it does not evaluate successfully if (inherits(try(facet, silent = TRUE), "try-error")) { From bdf1e2fb2dafc93134ea3083fb8dc3b4061637c7 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Thu, 20 Nov 2025 20:26:27 -0800 Subject: [PATCH 2/7] tests --- ...h_hline.svg => pointrange_with_layers.svg} | 2 + .../pointrange_with_layers_flipped.svg | 84 +++++++++++++++++++ inst/tinytest/test-type_pointrange.R | 18 +++- 3 files changed, 102 insertions(+), 2 deletions(-) rename inst/tinytest/_tinysnapshot/{pointrange_with_hline.svg => pointrange_with_layers.svg} (95%) create mode 100644 inst/tinytest/_tinysnapshot/pointrange_with_layers_flipped.svg diff --git a/inst/tinytest/_tinysnapshot/pointrange_with_hline.svg b/inst/tinytest/_tinysnapshot/pointrange_with_layers.svg similarity index 95% rename from inst/tinytest/_tinysnapshot/pointrange_with_hline.svg rename to inst/tinytest/_tinysnapshot/pointrange_with_layers.svg index d96fa8ae..fc8092ca 100644 --- a/inst/tinytest/_tinysnapshot/pointrange_with_hline.svg +++ b/inst/tinytest/_tinysnapshot/pointrange_with_layers.svg @@ -76,6 +76,8 @@ + + diff --git a/inst/tinytest/_tinysnapshot/pointrange_with_layers_flipped.svg b/inst/tinytest/_tinysnapshot/pointrange_with_layers_flipped.svg new file mode 100644 index 00000000..ee68be15 --- /dev/null +++ b/inst/tinytest/_tinysnapshot/pointrange_with_layers_flipped.svg @@ -0,0 +1,84 @@ + + + + + + + + + + + + + +y +x + + + + + + +-10 +0 +10 +20 +30 + + + + + +(Intercept) +hp +factor(cyl)6 +factor(cyl)8 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/inst/tinytest/test-type_pointrange.R b/inst/tinytest/test-type_pointrange.R index dad8f77e..7b0df176 100644 --- a/inst/tinytest/test-type_pointrange.R +++ b/inst/tinytest/test-type_pointrange.R @@ -34,7 +34,7 @@ fun = function() { } expect_snapshot_plot(fun, label = "pointrange_errorbar") -# issue 511: adding hline to coefplot +# issues #511 & #516: adding layers to coefplot fun = function() { tinyplot( y ~ x, ymin = ymin, ymax = ymax, @@ -42,9 +42,23 @@ fun = function() { type = "pointrange", theme = "basic" ) + tinyplot_add(type = "ribbon") tinyplot_add(type = "hline", lty = 2) } -expect_snapshot_plot(fun, label = "pointrange_with_hline") +expect_snapshot_plot(fun, label = "pointrange_with_layers") + +fun = function() { + tinyplot( + y ~ x, ymin = ymin, ymax = ymax, + data = coefs, + type = "pointrange", + theme = "basic", + flip = TRUE + ) + tinyplot_add(type = "ribbon") + tinyplot_add(type = "hline", lty = 2) +} +expect_snapshot_plot(fun, label = "pointrange_with_layers_flipped") # Issue #406: dodge pointrange and errorbar models = list( From 5cbcb1dd5b11e8a8edcc85c13ab8ed0b3a2e815d Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Thu, 20 Nov 2025 20:36:45 -0800 Subject: [PATCH 3/7] news --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index 2efdb0e4..55b05fa8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -40,6 +40,10 @@ where the formatting is also better._ axis limits for secondary plot layers. (#513 @grantmcdermott) - Fixed lazy evaluation bug where `legend` passed as a symbol through S3 methods (e.g., `tinyplot.foo`) would fail. (#515 @grantmcdermott) +- Add layers, particularly from `tinyplot_add()`, should now respect the x-axis + order of the original plot layer. This should ensure that we don't end up with + misaligned layers. For example, when adding a ribbon on top of an errorbar + plot. (#517 @grantmcdermott) ### Documentation From 031b216f58004edaeda44b59d53b9637c8bef002 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Thu, 20 Nov 2025 20:44:43 -0800 Subject: [PATCH 4/7] move to separate section --- R/tinyplot.R | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/R/tinyplot.R b/R/tinyplot.R index 8a47f725..97b29017 100644 --- a/R/tinyplot.R +++ b/R/tinyplot.R @@ -1182,11 +1182,11 @@ tinyplot.default = function( # - ## split and draw datapoints ----- + ## layering axis alignment ----- # - # layering gotcha for ordered factors along the x-axis (e.g., when adding - # lines or ribbons on top of errorbars) + # ensure added layers respect the x-axis order of the original plot layer + # (e.g., when adding lines or ribbons on top of errorbars) if (!add) { # keep track of the x(y)labs from the original (1st) layer assign("xlabs", xlabs, envir = get(".tinyplot_env", envir = parent.env(environment()))) @@ -1225,6 +1225,10 @@ tinyplot.default = function( } } + # + ## split and draw datapoints ----- + # + # Finally, we can draw all of the plot elements (points, lines, etc.) # We'll do this via a nested loops: # 1) Outer loop over facets From 24783654ec2287e162333f725a3cbc752df80ece Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Thu, 20 Nov 2025 20:45:17 -0800 Subject: [PATCH 5/7] spacing --- R/tinyplot.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/tinyplot.R b/R/tinyplot.R index 97b29017..1f619a4c 100644 --- a/R/tinyplot.R +++ b/R/tinyplot.R @@ -1225,6 +1225,7 @@ tinyplot.default = function( } } + # ## split and draw datapoints ----- # From 38b7a3fa605299e27eec5ab9762886246ff24bd9 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Thu, 20 Nov 2025 21:40:05 -0800 Subject: [PATCH 6/7] gotcha where ancilliary vars affect order --- R/tinyplot.R | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/R/tinyplot.R b/R/tinyplot.R index 1f619a4c..6c764633 100644 --- a/R/tinyplot.R +++ b/R/tinyplot.R @@ -1196,12 +1196,14 @@ tinyplot.default = function( # aside: we need some extra accounting for flipped plots (x -> y) if (!flip) { labs_orig = "xlabs" - labs_new = xlabs - dp_var = "x" + labs_layer = xlabs + dp_var = "x" + dp_ovars = c("rowid", "xmin", "xmax") } else { labs_orig = "ylabs" - labs_new = ylabs + labs_layer = ylabs dp_var = "y" + dp_ovars = c("rowid", "ymin", "ymax") } # retrieve the relevant axis labs from the original layer (and we only care # if it's not null) @@ -1215,17 +1217,23 @@ tinyplot.default = function( datapoints[[dp_var]] } ) - } else if (!is.null(names(labs_new))) { + } else if (!is.null(names(labs_layer))) { # case 2: match implicit integer -> label mapping (e.g., lines added to errorbars) - if (setequal(names(labs_new), names(labs_orig))) { - datapoints[[dp_var]] = labs_orig[names(labs_new)[datapoints[[dp_var]]]] + if (setequal(names(labs_layer), names(labs_orig))) { + orig_order = labs_orig[names(labs_layer)[datapoints[[dp_var]]]] + dp_var_layer = datapoints[[dp_var]] + datapoints[[dp_var]] = orig_order + # it's a bit of a pain, but we also have to adjust some ancillary vars + for (dov in dp_ovars) { + if (identical(datapoints[[dov]], dp_var_layer)) datapoints[[dov]] = orig_order + } datapoints = datapoints[order(datapoints[[dp_var]]), ] } } } } - + # ## split and draw datapoints ----- # From f4ff7d973582c280378f05aaf159259d24d4a5af Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Thu, 20 Nov 2025 21:43:20 -0800 Subject: [PATCH 7/7] add test for reverse case --- .../_tinysnapshot/ribbon_with_errorbar.svg | 91 +++++++++++++++++++ inst/tinytest/test-type_pointrange.R | 12 +++ 2 files changed, 103 insertions(+) create mode 100644 inst/tinytest/_tinysnapshot/ribbon_with_errorbar.svg diff --git a/inst/tinytest/_tinysnapshot/ribbon_with_errorbar.svg b/inst/tinytest/_tinysnapshot/ribbon_with_errorbar.svg new file mode 100644 index 00000000..86917878 --- /dev/null +++ b/inst/tinytest/_tinysnapshot/ribbon_with_errorbar.svg @@ -0,0 +1,91 @@ + + + + + + + + + + + + + +x +y + + + + + +(Intercept) +factor(cyl)6 +factor(cyl)8 +hp + + + + + + +-10 +0 +10 +20 +30 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/inst/tinytest/test-type_pointrange.R b/inst/tinytest/test-type_pointrange.R index 7b0df176..06d8dd80 100644 --- a/inst/tinytest/test-type_pointrange.R +++ b/inst/tinytest/test-type_pointrange.R @@ -47,6 +47,18 @@ fun = function() { } expect_snapshot_plot(fun, label = "pointrange_with_layers") +# test the reverse too (i.e., adding errorbars on a ribbon) +fun = function() { + tinyplot( + y ~ x, ymin = ymin, ymax = ymax, + data = coefs, + type = "ribbon", + theme = "basic" + ) + tinyplot_add(type = "errorbar") +} +expect_snapshot_plot(fun, label = "ribbon_with_errorbar") + fun = function() { tinyplot( y ~ x, ymin = ymin, ymax = ymax,