From 0e692893956f4ca9e1cc3c7195fb375e44ce3518 Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Fri, 19 Jun 2026 15:05:23 -0700 Subject: [PATCH 1/2] refactor: centralize spineplot special-cases via type_axes_hints Replace six scattered hardcoded `type == "spineplot"` checks with semantic flags the type declares for itself. data_spineplot() now writes a type_axes_hints list (self_axes, rhs_axis, proportional_lim, legend_fill_from_col, draw_empty_facet) to settings via the same env2env write-back pattern used by group_offsets. The main pipeline reads these flags instead of matching the type string. Sites converted: - R/lim.R: proportional_lim (suppress axis-break range expansion) - R/tinyplot.R: self_axes (dynmar tick-row margin, #635) + draw_empty_facet - R/facet.R: rhs_axis (omar[4] RHS-axis margin, x2; drops the FIXMEs) - R/legend.R: rhs_axis (outer-legend bump) + legend_fill_from_col (pt.bg) - R/legend_multi.R: thread type_axes_hints through the multi-legend path type_axes_hints is threaded through the exported draw_legend() (new NULL default + @param) -> build_legend_env() -> stored on legend_env. Pure refactor: all five flags are TRUE for spineplot, NULL elsewhere, so every site evaluates identically. Verified byte-identical renders across simple, flip, facet, by-group, outer-legend, and xlab/ylab=NA (#635) cases. Adds a spineplot_legend_outer_right snapshot to cover the RHS legend-margin path. --- R/facet.R | 7 +++++-- R/legend.R | 14 +++++++++++--- R/legend_multi.R | 3 +++ R/lim.R | 9 +++++---- R/tinyplot.R | 12 +++++++++--- R/type_spineplot.R | 17 +++++++++++++++-- inst/tinytest/test-type_spineplot.R | 10 ++++++++++ man/build_legend_env.Rd | 1 + man/draw_legend.Rd | 5 +++++ man/facet.Rd | 1 + 10 files changed, 65 insertions(+), 14 deletions(-) diff --git a/R/facet.R b/R/facet.R index a9b73a373..1b10f51f3 100644 --- a/R/facet.R +++ b/R/facet.R @@ -35,6 +35,7 @@ draw_facet_window = function( sub, cap, type, + type_axes_hints = NULL, xlab, x, xmax, xmin, ylab, @@ -188,7 +189,8 @@ draw_facet_window = function( } } - if (type == "spineplot") omar[4] = 2.1 # FIXME catch for spineplot RHS axis labs + # reserve RHS margin for types with a secondary axis (e.g. spineplot) + if (isTRUE(type_axes_hints[["rhs_axis"]])) omar[4] = 2.1 # FIXME: Is this causing issues for lhs legends with facet_grid? # catch for missing rhs legend @@ -227,7 +229,8 @@ draw_facet_window = function( # Tick-label *width/height* (whtsbp) is added further below. side.sub = get_tpar("side.sub", tpar_list = tpars, default = 3) omar = dynmar_computed - if (type == "spineplot") omar[4] = 2.1 # FIXME catch for spineplot RHS axis labs + # reserve RHS margin for types with a secondary axis (e.g. spineplot) + if (isTRUE(type_axes_hints[["rhs_axis"]])) omar[4] = 2.1 if (par("las") %in% 1:2) { # extra whitespace bump on the y axis ## overrides for ridge and some types that use integer spacing with (named) axis labels ## FXIME diff --git a/R/legend.R b/R/legend.R index b7553cc7a..5465b2dc1 100644 --- a/R/legend.R +++ b/R/legend.R @@ -82,8 +82,9 @@ legend_outer_margins = function(legend_env, apply = TRUE) { # Step 1: Prepare margins before measuring if (legend_env$outer_side) { - # Extra bump for spineplot if outer_right legend (to accommodate secondary y-axis) - if (identical(legend_env$type, "spineplot")) { + # Extra bump for types with a secondary (RHS) axis when an outer legend is + # present, to accommodate that axis (e.g. spineplot). + if (isTRUE(legend_env$type_axes_hints[["rhs_axis"]])) { lmar[1] = lmar[1] + 1.1 } @@ -600,7 +601,7 @@ build_legend_args = function( } # Special pt.bg handling for types that need color-based fills - if (identical(type, "spineplot")) { + if (isTRUE(legend_env[["type_axes_hints"]][["legend_fill_from_col"]])) { legend_args[["pt.bg"]] = legend_args[["pt.bg"]] %||% legend_args[["col"]] } else if (identical(type, "ridge") && isFALSE(gradient)) { legend_args[["pt.bg"]] = legend_args[["pt.bg"]] %||% sapply(legend_args[["col"]], function(ccol) seq_palette(ccol, n = 2)[2]) @@ -739,6 +740,7 @@ build_legend_env = function( # Visual aesthetics type, + type_axes_hints = NULL, pch, lty, lwd, @@ -761,6 +763,7 @@ build_legend_env = function( # Initialize metadata legend_env$gradient = gradient legend_env$type = type + legend_env$type_axes_hints = type_axes_hints legend_env$has_sub = has_sub legend_env$has_cap = has_cap legend_env$cap_text = cap_text @@ -821,6 +824,9 @@ build_legend_env = function( #' @param labeller Character or function for formatting the labels (`lgnd_labs`). #' Passed down to [`tinylabel`]. #' @param type Plotting type(s), passed down from [tinyplot]. +#' @param type_axes_hints Optional named list of axes/legend behaviour flags that +#' a plot type declares for itself (e.g. `rhs_axis` for a secondary right-hand +#' axis). Passed down from [tinyplot]; defaults to `NULL`. #' @param pch Plotting character(s), passed down from [tinyplot]. #' @param lty Plotting linetype(s), passed down from [tinyplot]. #' @param lwd Plotting line width(s), passed down from [tinyplot]. @@ -925,6 +931,7 @@ draw_legend = function( lgnd_labs = NULL, labeller = NULL, type = NULL, + type_axes_hints = NULL, pch = NULL, lty = NULL, lwd = NULL, @@ -976,6 +983,7 @@ draw_legend = function( # Visual aesthetics type = type, + type_axes_hints = type_axes_hints, pch = pch, lty = lty, lwd = lwd, diff --git a/R/legend_multi.R b/R/legend_multi.R index 789734d7d..68b6c3b98 100644 --- a/R/legend_multi.R +++ b/R/legend_multi.R @@ -19,6 +19,7 @@ prepare_legend_multi = function(settings) { "by_dep", "lgnd_labs", "type", + "type_axes_hints", "pch", "lty", "lwd", @@ -45,6 +46,7 @@ prepare_legend_multi = function(settings) { by_dep = by_dep, lgnd_labs = lgnd_labs, type = type, + type_axes_hints = type_axes_hints, pch = pch, lty = lty, lwd = lwd, @@ -64,6 +66,7 @@ prepare_legend_multi = function(settings) { ), lgnd_labs = names(bubble_cex), type = type, + type_axes_hints = type_axes_hints, pch = pch, lty = lty, lwd = lwd, diff --git a/R/lim.R b/R/lim.R index 7eb13aad0..034c5ff52 100644 --- a/R/lim.R +++ b/R/lim.R @@ -5,9 +5,9 @@ lim_args = function(settings) { settings, environment(), c( - "xaxb", "xlabs", "xlim", "null_xlim", + "xaxb", "xlabs", "xlim", "null_xlim", "yaxb", "ylabs", "ylim", "null_ylim", - "datapoints", "type" + "datapoints", "type", "type_axes_hints" ) ) @@ -37,8 +37,9 @@ lim_args = function(settings) { xlim = xlim + c(-0.5, 0.5) } - if (null_xlim && !is.null(xaxb) && type != "spineplot") xlim = range(c(xlim, xaxb)) - if (null_ylim && !is.null(yaxb) && type != "spineplot") ylim = range(c(ylim, yaxb)) + prop_lim = isTRUE(type_axes_hints[["proportional_lim"]]) + if (null_xlim && !is.null(xaxb) && !prop_lim) xlim = range(c(xlim, xaxb)) + if (null_ylim && !is.null(yaxb) && !prop_lim) ylim = range(c(ylim, yaxb)) # update settings env2env( diff --git a/R/tinyplot.R b/R/tinyplot.R index d21be892d..bdd11c7ee 100644 --- a/R/tinyplot.R +++ b/R/tinyplot.R @@ -869,7 +869,8 @@ tinyplot.default = function( group_offsets = NULL, offsets_axis = NULL, sub = sub, - type_info = list() # pass type-specific info from type_data to type_draw + type_info = list(), # pass type-specific info from type_data to type_draw + type_axes_hints = NULL # axes/legend behaviour a type declares for itself ) settings = new.env() @@ -1063,7 +1064,7 @@ tinyplot.default = function( # draw their own in the same place (e.g. spineplot category + numeric labels # via spine_axis()). Their tick-row margin must still be reserved, else the # self-drawn labels clip when xlab/ylab = NA makes label_extent = 0 (#635). - .self_axis = identical(type, "spineplot") + .self_axis = isTRUE(type_axes_hints[["self_axes"]]) .dyn = c( dynmar_side(1, xlab, main = main, sub = sub, @@ -1157,6 +1158,7 @@ tinyplot.default = function( by_dep = by_dep, lgnd_labs = lgnd_labs, type = type, + type_axes_hints = type_axes_hints, pch = pch, lty = lty, lwd = lwd, @@ -1330,6 +1332,7 @@ tinyplot.default = function( sub = sub, cap = cap, type = type, + type_axes_hints = type_axes_hints, xlab = xlab, x = x, xmax = xmax, xmin = xmin, ylab = ylab, @@ -1361,6 +1364,7 @@ tinyplot.default = function( sub = sub, cap = cap, type = type, + type_axes_hints = type_axes_hints, xlab = xlab, x = datapoints$x, xmax = datapoints$xmax, xmin = datapoints$xmin, ylab = ylab, @@ -1463,7 +1467,9 @@ tinyplot.default = function( # empty plot flag empty_plot = FALSE - if (isTRUE(empty) || isTRUE(type == "n") || ((length(ix) == 0) && !(type %in% c("histogram", "hist", "rect", "segments", "spineplot")))) { + draws_empty = type %in% c("histogram", "hist", "rect", "segments") || + isTRUE(type_axes_hints[["draw_empty_facet"]]) + if (isTRUE(empty) || isTRUE(type == "n") || ((length(ix) == 0) && !draws_empty)) { empty_plot = TRUE } diff --git a/R/type_spineplot.R b/R/type_spineplot.R index bca7aebff..e2ef915b8 100644 --- a/R/type_spineplot.R +++ b/R/type_spineplot.R @@ -283,11 +283,24 @@ data_spineplot = function(off = NULL, breaks = NULL, xlevels = xlevels, ylevels settings$legend_args[["pt.lwd"]] = settings$legend_args[["pt.lwd"]] %||% 0 settings$legend_args[["y.intersp"]] = settings$legend_args[["y.intersp"]] %||% 1.25 settings$legend_args[["seg.len"]] = settings$legend_args[["seg.len"]] %||% 1.25 - + + # Declare this type's axes/legend behaviour so the main pipeline can read + # semantic flags instead of hardcoding `type == "spineplot"` checks. + # A spineplot suppresses the standard axes (xaxt/yaxt = "n") because it + # draws its own (category + numeric labels, plus a secondary RHS axis) + # via spine_axis(), and uses proportional [0, 1] limits. + type_axes_hints = list( + self_axes = TRUE, # draws own tick-row axes despite xaxt/yaxt = "n" + rhs_axis = TRUE, # secondary right-hand axis (reserve margin) + proportional_lim = TRUE, # [0, 1] limits; don't expand to axis breaks + legend_fill_from_col = TRUE, # legend swatch pt.bg defaults from col + draw_empty_facet = TRUE # can draw with zero per-group rows + ) + env2env(environment(), settings, c( "x", "y", "ymin", "ymax", "xmin", "xmax", "col", "bg", "datapoints", "by", "facet", "axes", "frame.plot", "xaxt", "yaxt", "xaxs", "yaxs", - "ylabs", "type_info", "facet.args" + "ylabs", "type_info", "facet.args", "type_axes_hints" )) } diff --git a/inst/tinytest/test-type_spineplot.R b/inst/tinytest/test-type_spineplot.R index bf96e7a19..3fbeda803 100644 --- a/inst/tinytest/test-type_spineplot.R +++ b/inst/tinytest/test-type_spineplot.R @@ -105,3 +105,13 @@ f = function() { theme = "dynamic", type = "spineplot", xlab = NA) } expect_snapshot_plot(f, label = "spineplot_xlab_na_issue635") + +# +## outer legend exercises the secondary (RHS) axis margin bump (type_axes_hints +## rhs_axis); guards the spineplot legend-margin path during the #635 refactor + +f = function() { + tinyplot(Species ~ Sepal.Width, data = iris, + type = "spineplot", legend = "right!") +} +expect_snapshot_plot(f, label = "spineplot_legend_outer_right") diff --git a/man/build_legend_env.Rd b/man/build_legend_env.Rd index 684e30d2e..b3e21904c 100644 --- a/man/build_legend_env.Rd +++ b/man/build_legend_env.Rd @@ -11,6 +11,7 @@ build_legend_env( lgnd_labs, labeller = NULL, type, + type_axes_hints = NULL, pch, lty, lwd, diff --git a/man/draw_legend.Rd b/man/draw_legend.Rd index c9b16353c..6709c8211 100644 --- a/man/draw_legend.Rd +++ b/man/draw_legend.Rd @@ -11,6 +11,7 @@ draw_legend( lgnd_labs = NULL, labeller = NULL, type = NULL, + type_axes_hints = NULL, pch = NULL, lty = NULL, lwd = NULL, @@ -43,6 +44,10 @@ Passed down to \code{\link{tinylabel}}.} \item{type}{Plotting type(s), passed down from \link{tinyplot}.} +\item{type_axes_hints}{Optional named list of axes/legend behaviour flags that +a plot type declares for itself (e.g. \code{rhs_axis} for a secondary right-hand +axis). Passed down from \link{tinyplot}; defaults to \code{NULL}.} + \item{pch}{Plotting character(s), passed down from \link{tinyplot}.} \item{lty}{Plotting linetype(s), passed down from \link{tinyplot}.} diff --git a/man/facet.Rd b/man/facet.Rd index 1be2b97ff..03f663d3f 100644 --- a/man/facet.Rd +++ b/man/facet.Rd @@ -51,6 +51,7 @@ draw_facet_window( sub, cap, type, + type_axes_hints = NULL, xlab, x, xmax, From da301f909b2bbfddd96fdcd9163a8bfce64c841c Mon Sep 17 00:00:00 2001 From: Grant McDermott Date: Fri, 19 Jun 2026 15:26:18 -0700 Subject: [PATCH 2/2] test: drop redundant spineplot outer-legend snapshot The default legend position is already "right!" (outer), so the existing spineplot_facet_by test exercises the outer_side + rhs_axis legend-margin branch. A dedicated outer-right test added no new coverage. --- inst/tinytest/test-type_spineplot.R | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/inst/tinytest/test-type_spineplot.R b/inst/tinytest/test-type_spineplot.R index 3fbeda803..bf96e7a19 100644 --- a/inst/tinytest/test-type_spineplot.R +++ b/inst/tinytest/test-type_spineplot.R @@ -105,13 +105,3 @@ f = function() { theme = "dynamic", type = "spineplot", xlab = NA) } expect_snapshot_plot(f, label = "spineplot_xlab_na_issue635") - -# -## outer legend exercises the secondary (RHS) axis margin bump (type_axes_hints -## rhs_axis); guards the spineplot legend-margin path during the #635 refactor - -f = function() { - tinyplot(Species ~ Sepal.Width, data = iris, - type = "spineplot", legend = "right!") -} -expect_snapshot_plot(f, label = "spineplot_legend_outer_right")