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 diff --git a/R/tinyplot.R b/R/tinyplot.R index 7785bcdc..6c764633 100644 --- a/R/tinyplot.R +++ b/R/tinyplot.R @@ -913,7 +913,6 @@ tinyplot.default = function( # ## legends ----- # - # browser() # legend labels ncolors = length(col) @@ -1182,6 +1181,59 @@ tinyplot.default = function( list2env(facet_window_args, environment()) + # + ## layering axis alignment ----- + # + + # 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()))) + 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_layer = xlabs + dp_var = "x" + dp_ovars = c("rowid", "xmin", "xmax") + } else { + labs_orig = "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) + 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_layer))) { + # case 2: match implicit integer -> label mapping (e.g., lines added to errorbars) + 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 ----- # @@ -1397,7 +1449,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")) { 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/_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 dad8f77e..06d8dd80 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,35 @@ 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") + +# 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, + 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(