From e4683d8843e7c2aa901c7b5a5e895a86fe73ec21 Mon Sep 17 00:00:00 2001 From: Paul Schmidt Date: Fri, 7 Aug 2026 13:01:13 +0200 Subject: [PATCH] ggdesplot: draw num/text labels inside cells when there is no fill With an empty formula LHS (no fill variable) ggdesplot() drew no tile, so the +-0.5 cell extent was never established and num/text labels on the outer cells were clipped at the panel border. Draw a transparent tile grid in that case, as the lattice path already does. Also assert the deliberate col.regions positional-fallback warning with expect_warning() instead of hiding it with suppressWarnings(). --- NEWS.md | 2 ++ R/ggdesplot.R | 8 ++++++++ tests/testthat/test_ggdesplot_fixes.R | 29 ++++++++++++++++++++++++--- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index dbe52ca..71d89ef 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,6 +12,8 @@ * `ggdesplot()` named `col.regions` and `col.text` now fall back to positional matching (as the warning states) when some factor levels are unnamed, instead of leaving cells uncolored. (P.Schmidt) +* `ggdesplot()` now positions `num`/`text` labels correctly when the formula has no left-hand side (no fill variable, e.g. `~ col*row | block`). Previously the labels on the outer cells were clipped at the panel border because, without a filled tile, the half-cell extent was never established. (P.Schmidt) + * Switch to MIT license. * Documentation pages now created via Github Actions. diff --git a/R/ggdesplot.R b/R/ggdesplot.R index 7b1d402..57582f4 100644 --- a/R/ggdesplot.R +++ b/R/ggdesplot.R @@ -488,6 +488,14 @@ ggdesplot <- function(data, # Note, cells with a missing value are left empty instead of being filled # with the ggplot2 default grey50, which is hard to tell from the lightgray # midpoint of RedGrayBlue. The lattice version leaves such cells empty too. + if(fill.type=="none") + # No fill variable (empty formula LHS). Draw an invisible tile grid anyway + # so the +-0.5 cell extent is established; otherwise num/text labels on the + # outer cells sit on the panel border and are clipped. The lattice path + # reserves this extent regardless of fill. + out <- out + + geom_tile(fill = "transparent") + if(fill.type=="num") out <- out + #geom_tile(aes_string(fill = fill.string)) + diff --git a/tests/testthat/test_ggdesplot_fixes.R b/tests/testthat/test_ggdesplot_fixes.R index 7714db7..76dea95 100644 --- a/tests/testthat/test_ggdesplot_fixes.R +++ b/tests/testthat/test_ggdesplot_fixes.R @@ -32,13 +32,36 @@ test_that("ggdesplot does not add a spurious colour legend without 'col'", { }) test_that("ggdesplot named col.regions fallback colours every level", { - p <- suppressWarnings( - ggdesplot(partial, rep ~ col * row, col.regions = c(R1 = "red", R2 = "blue"))) + # the names cover only R1/R2 of four levels: a deliberate warning fires and + # the code falls back to positional matching. Assert the warning (not suppress + # it) and then check the fallback still colours every tile (no grey NA). + expect_warning( + p <- ggdesplot(partial, rep ~ col * row, col.regions = c(R1 = "red", R2 = "blue")), + "positional" + ) b <- ggplot2::ggplot_build(p) - # no tile should be left unfilled (grey NA) by the positional fallback expect_false(any(is.na(b$data[[1]]$fill))) }) +test_that("ggdesplot with no fill variable keeps num/text labels inside the panel", { + # Empty LHS: cells carry only numbers (num=), no fill. Without a tile to + # establish the +-0.5 cell extent, boundary labels are drawn on the panel + # edge and get clipped (the num= plot looked wrong). A transparent tile grid, + # as in the lattice path, must provide that extent. + blk <- expand.grid(col = 1:2, row = 1:3, block = c("B1", "B2")) + blk$block <- factor(blk$block) + blk$g <- factor(rep(1:3, length.out = nrow(blk))) + p <- ggdesplot(blk, ~ col * row | block, num = g) + b <- ggplot2::ggplot_build(p) + rng <- b$layout$panel_params[[1]]$x.range + # the drawn range must reach half a cell beyond the outer column centres, + # otherwise a label at col 1 or col 2 sits on the panel border + expect_lte(rng[1], 0.5) + expect_gte(rng[2], 2.5) + # the extent comes from a (transparent) tile layer, not from the text alone + expect_true(any(vapply(p$layers, function(l) inherits(l$geom, "GeomTile"), logical(1)))) +}) + test_that("ggdesplot facets on every conditioning variable", { p <- ggdesplot(twocond, y ~ x * row | site + rep) b <- ggplot2::ggplot_build(p)