Skip to content

[ggdesplot] fix spurious colour legend, named-palette fallback, and multi-facet#17

Open
SchmidtPaul wants to merge 1 commit into
kwstat:mainfrom
SchmidtPaul:fix/ggdesplot-colour-and-facets
Open

[ggdesplot] fix spurious colour legend, named-palette fallback, and multi-facet#17
SchmidtPaul wants to merge 1 commit into
kwstat:mainfrom
SchmidtPaul:fix/ggdesplot-colour-and-facets

Conversation

@SchmidtPaul

Copy link
Copy Markdown
Contributor

These are three independent fixes in ggdesplot(). The lattice desplot() is not affected by any of them - in each case desplot() already produces the correct result, and ggdesplot() now matches it.

All reprexes use agridat::besag.met / synthetic data only.


1. Spurious no_color legend

When text or num is supplied without col, an internal dummy factor (no_color) leaks in as a visible colour legend.

library(desplot)
data(besag.met, package = "agridat")
ggdesplot(besag.met, yield ~ col*row, text = gen)
#> before: a legend titled "no_color" with a single entry "1" appears
#>         above the yield colorbar
#> after:  only the yield colorbar is shown (matches desplot())

Cause: without col, the code sets col.string = "no_color"; data[[col.string]] <- factor(1) and then unconditionally adds scale_color_manual(). The colour key is now suppressed (guide = "none") unless col was actually supplied.


2. Named col.regions / col.text fallback leaves cells uncolored

With a partial named palette, the warning promises "falling back to positional matching", but unmatched levels are drawn grey.

library(desplot)
d <- expand.grid(col = 1:4, row = 1:4)
d$rep <- factor(paste0("R", rep(1:4, length.out = 16)))   # R1..R4
ggdesplot(d, rep ~ col*row, col.regions = c(R1 = "red", R2 = "blue"))
#> before: R3 and R4 cells are grey50 (NA) despite the "positional matching" warning
#> after:  all four levels are colored positionally (red, blue, red, blue),
#>         matching desplot()

Cause: rep(col.regions, length = fill.n) kept the vector names, so scale_fill_manual() still matched by name and dropped unmatched levels. Names are now stripped (as.vector()) so the documented positional fallback actually applies. Same fix for the col.text fallback.


3. Only the first conditioning variable was used

A formula with two conditioning variables silently facets on the first only; cells from the dropped factor overplot each other.

library(desplot)
d <- expand.grid(x = 1:3, row = 1:3, site = c("S1","S2"), rep = c("R1","R2"))
d$site <- factor(d$site); d$rep <- factor(d$rep)
d$y <- as.numeric(interaction(d$site, d$rep))            # constant within a panel

ggdesplot(d, y ~ x*row | site + rep)
#> before: 2 panels (site only), 18 tiles per panel -> rep overplotted
#> after:  4 panels (site x rep), 9 tiles per panel, matching desplot()

Cause: facet_wrap() used ff$cond[1] only. All conditioning variables are now combined into a single panel factor, so every conditioning variable is faceted. Single-variable formulas keep their original panel labels.


Tests for all three are added in tests/testthat/test_ggdesplot_fixes.R. R CMD check is clean (0 errors, 0 warnings).

Note: these were found during an LLM-assisted bug hunt of desplot.

…ulti-facet

Three issues in ggdesplot() (the lattice desplot() is unaffected):

- A dummy 'no_color' factor leaked in as a visible colour legend whenever
  'text' or 'num' was used without 'col'. The colour key is now suppressed
  unless 'col' is supplied.

- The named col.regions / col.text fallback kept vector names, so
  scale_*_manual() still matched by name and left unmatched levels uncolored.
  Names are stripped so the documented positional fallback actually applies.

- A formula like 'y ~ x*z | a + b' only faceted on the first conditioning
  variable; the rest were silently dropped and cells overplotted. All
  conditioning variables are now combined into the panel factor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant