Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 8 additions & 0 deletions R/ggdesplot.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)) +
Expand Down
29 changes: 26 additions & 3 deletions tests/testthat/test_ggdesplot_fixes.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading