added 2 commits
June 19, 2026 15:16Replace 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.
Follow-up to the spineplot pilot, addressing three gaps in how types declare behaviour to the generic pipeline. Remove the last generic-pipeline read of `type_info`. The grouped (`y_by`) legend-fill branch in prepare_legend() was guarded by a hardcoded `type == "spineplot"` check, giving spineplot two parallel legend-fill mechanisms. It now keys off the `legend_fills_from_col` hint. The fill resolution itself stays put: `col` is only the resolved group palette after by_aesthetics(), so it cannot move into data_spineplot(). Rename `type_axes_hints` to `type_hints`, since several flags (legend fill, empty-group drawing) are not axes concerns. Flags are renamed to property-style predicates for a consistent convention: self_axes -> draws_own_axes rhs_axis -> has_rhs_axis proportional_lim -> has_proportional_lim legend_fill_from_col -> legend_fills_from_col Replace the empty-plot type list with a data-driven check. Rather than asking which type this is, ask whether there is anything to draw: has_data = length(ix) > 0 || (length(ixmin) > 0 && length(ixmax) > 0) Requiring both xmin and xmax isolates the types that carry horizontal geometry there (rect, segments, histogram, spineplot) from those needing `x` alongside a y-extent (errorbar, pointrange, ribbon). This drops the `draw_empty_facet` flag and the hardcoded list together. That last change also fixes a latent bug: the old list drew a group whenever the type matched, even with nothing to draw. On grouped and faceted spineplots this redrew each facet's axes once per empty group (40 spine_axis() calls where 10 suffice), overplotting the self-drawn labels so they rendered too heavy. Two snapshots are updated to the corrected single-draw output.
Faceted plots suppress interior axes when frameless, since without a frame those axes float into the neighbouring panel and collide with its labels. The generic pipeline already did this via oxaxis/oyaxis, but three paths bypassed it. Introduce a single predicate, draw_facet_axis(), as the one place that answers "should this facet draw this axis?", and route every decision site through it. Previously the rule was inlined in facet.R and the self-drawing types reimplemented (or omitted) it. - spineplot drew its category axis in every facet. It sets axes = FALSE in its data fn, so the generic block is skipped entirely and all axis drawing happens in draw_spineplot(). Note the gate reads frame.plot from type_info, since data_spineplot() forces the settings copy FALSE while preserving the user's value. - ridge likewise drew its y-axis category labels unconditionally, plus the "ridge2" x-axis line. draw_ridge() gains named ifacet and facet_window_args params; they were already passed via `...`. - flipped plots gated args_x on oxaxis and args_y on oyaxis, i.e. by argument name rather than by resolved side. Since flip swaps xside/yside, a flipped frameless facet grid both duplicated the category axis and omitted every interior x-axis -- so panels were read against a neighbour's scale. Gating on the actual side fixes both. Free facets keep per-facet axes, as each panel has its own scale. Also adds the `axes` facet argument from #661: "all", "outer", or "none". This was previously only reachable as a side effect of frame.plot = FALSE, so `facet.args = list(axes = "outer")` now drops interior axes while keeping frames. Unset preserves existing behaviour. An explicit "none" overrides the free-facet exemption. Verified byte-identical renders for framed, free, single-facet, facet_grid, nrow=1 and flipped-framed cases; only the frameless cases change.
Faceted spineplots overlapped their category labels with the
neighbouring panel under framed themes (e.g. tinytheme("clean")).
The tick-label width is measured once and added to both omar and fmar,
but the nmar/noma split then hands it to the *outer* margin:
omar[2] = 6.37 (base + ~2.85 label width)
noma[2] = 4.77 <- label space, figure left edge only
nmar[2] = 1.6 <- every facet's own left gap
That is right when only the leftmost facet draws a y axis. Under a framed
theme every facet draws one, into 1.6 lines when the labels need 2.85, so
they spill leftwards into the neighbour. Keep the fmar bump (rather than
releasing it to the outer margin) whenever an interior facet will draw
that axis, decided by the same draw_facet_axis() predicate that governs
the drawing itself, so margin and axis cannot disagree.
Two supporting details:
- data_spineplot() forces frame.plot = FALSE so the pipeline skips the
box that draw_spineplot() draws itself. draw_facet_window() therefore
saw FALSE under *both* framed and frameless themes and subtracted the
label width in both cases. Surface the user's real choice via a new
`framed` hint and key the margin logic off that.
- The subtraction condition keeps its original frameless/free guards, so
this can only retain space that was previously released, never release
more.
Costs some panel width, since the labels now occupy each facet's margin
rather than one shared outer margin. Verified byte-identical renders for
points, boxplot, ridge and frameless spineplot; only framed faceted
spineplots change.
The pt.bg block in build_legend_args() still matched two types by name,
sitting immediately beside the already-migrated legend_fills_from_col
check. Convert both so the generic legend code no longer needs to know
which types exist.
Splitting them out revealed that legend_fills_from_col was conflating two
independent behaviours: deriving the swatch fill from `col`, and drawing
the swatch border in par("fg"). Spineplot wants both -- its tiles abut
with no gap, so a foreground border reads better -- but hexbin wants only
the fill. Hence three hints rather than two:
legend_fills_from_col spineplot, hexbin
legend_fills_from_seq_palette ridge (a lighter step of the ramp)
legend_border_fg spineplot
The border check now runs independently of the fill branch, which is what
lets hexbin reuse the fill behaviour without inheriting the border.
Pure refactor: verified byte-identical legend renders across spineplot
(y_by, y_by + lighten, by, plain), ridge (by, gradient, plain), hexbin
(discrete, count) and area-type controls (barplot, boxplot, rect). Also
confirmed by trace that each type declares exactly the expected hints and
that non-migrated types declare none.
The "which labels will the y axis draw?" cascade was duplicated three
times: once in tinyplot.default()'s dynmar block and twice in
draw_facet_window() (faceted and non-faceted branches). Each copy carried
its own `type == "ridge"` and `type == "boxplot" && flip` checks, making
them the largest remaining source of hardcoded type strings in the
pipeline.
Extract just the label-selection part as y_axis_labels(), returning NULL
when the axis is numeric so each caller keeps its own fallback.
Deliberately narrow: only the three genuinely identical branches move. The
surrounding whtsbp arithmetic is left alone because the three copies have
diverged in ways that are not obviously intentional -- differing las
source (get_tpar vs par), strwidth cex (.cex_axis vs par("cex.axis")),
guards on the computed bump (is.finite vs > 0 vs none), the
degenerate-range catch (present in two of three), and cex_fct_adj scaling.
Harmonising those would move margins on many snapshots and belongs in its
own change; see SCRATCH/facet-margin-slack.md.
The faceted copy also has a free-facet branch that measures every facet
and keeps the widest tick set. That stays inline, now nested under the
NULL fallback so branch precedence is unchanged.
Pure refactor. Verified equivalence of old and new selection over a truth
table including the empty-but-non-NULL ylabs edge case, plus byte-identical
renders across ridge (plain/faceted/ridge theme), flipped and unflipped
boxplot, spineplot, numeric axes, free facets, las = 2 and a degenerate
(zero-range) y axis.
d8b8809 used NULL as y_axis_labels()'s "not a categorical axis" signal, but `levels(y)` is itself NULL for a ridge plot over a *numeric* y (e.g. `am ~ mpg`). The caller then fell through and substituted axisTicks(), measuring ~0.028 figure units of tick-label width where the original cascade measured max(strwidth(NULL)) = 0, so the y labels were pushed away from the axis title. Wrap the result in a one-element list, so an empty label set stays distinguishable from "this type doesn't put categories on the y axis". Caught by the ridge_gradient_facet* and spineplot_facet_by_fancy snapshots. My earlier equivalence check missed it because every ridge case in the truth table used a factor y; the numeric-y case is now covered there too.
`axes = "t"` (ticks) wrongly dropped interior facet axis labels, e.g.
tinyplot(Survived ~ Sex | Class, facet = "by", data = ttnc,
type = type_spineplot(weights = ttnc$Freq), axes = "t", lwd = 0)
draw_facet_axis() keys the "outer facets only" rule off framing, on the
grounds that an unframed interior axis floats free and collides with the
neighbouring panel. But `frame.plot` is only a proxy for that, and
sanitize_axes() derives it as all(c(xaxt, yaxt) %in% c("s", "a")) -- so
"t" reports FALSE despite drawing tick marks that anchor the labels
perfectly well. Only "l" (labels) and "n" (none) are genuinely bare.
Add facet_axes_framed() as the single place that answers "are interior
tick labels anchored?", and route all three framing derivations through it
(the generic pipeline plus the two self-drawing types, which each computed
this independently and so shared the bug).
Verified the reported call now renders byte-identical to 99b34a3 , i.e.
before the outer-facet rule existed. Call counts confirm the intended
matrix is preserved: per-facet for framed and for axes="t"; outer-only for
axes="labels" and frameless themes (#660); and facet.args = list(axes =
"outer") still forces outer-only under axes="t". No change to the earlier
regression set (8/8 identical).
The deeper fix is to drop `frame.plot` from this decision in favour of an
explicit "would inner axes float?" flag, but that is a wider behaviour
change; noted in SCRATCH/facet-margin-slack.md.
grantmcdermott
changed the title
Centralize type-specific logic via self-declared flags (re: #543)
Give custom types control over plot machinery via type_hints
This was referenced
Jul 31, 2026Merged
Merged