From 47b573eb23b4a67245243691fca314d8827d3cf8 Mon Sep 17 00:00:00 2001 From: Utkarsh Date: Sat, 14 Mar 2026 00:22:41 +0530 Subject: [PATCH 1/4] Make diagnostic color matching explicit for neff --- NEWS.md | 1 + R/mcmc-diagnostics.R | 10 ++++++++-- tests/testthat/test-mcmc-diagnostics.R | 7 +++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 25c323e39..ceacec8fb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ * New functions `mcmc_dots` and `mcmc_dots_by_chain` for dot plots of MCMC draws by @behramulukir (#402) * Default to `quantiles=100` for all dot plots by @behramulukir (#402) +* Make diagnostic color scale helpers handle `"neff"` and `"neff_ratio"` explicitly, avoiding reliance on partial matching. # bayesplot 1.15.0 diff --git a/R/mcmc-diagnostics.R b/R/mcmc-diagnostics.R index 3dc1b3249..b37a0f9da 100644 --- a/R/mcmc-diagnostics.R +++ b/R/mcmc-diagnostics.R @@ -429,7 +429,10 @@ scale_fill_diagnostic <- function(diagnostic = c("rhat", "neff")) { diagnostic_color_scale <- function(diagnostic = c("rhat", "neff_ratio"), aesthetic = c("color", "fill")) { - diagnostic <- match.arg(diagnostic) + diagnostic <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) + if (diagnostic == "neff") { + diagnostic <- "neff_ratio" + } aesthetic <- match.arg(aesthetic) dc <- diagnostic_colors(diagnostic, aesthetic) do.call( @@ -445,7 +448,10 @@ diagnostic_color_scale <- function(diagnostic = c("rhat", "neff_ratio"), diagnostic_colors <- function(diagnostic = c("rhat", "neff_ratio"), aesthetic = c("color", "fill")) { - diagnostic <- match.arg(diagnostic) + diagnostic <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) + if (diagnostic == "neff") { + diagnostic <- "neff_ratio" + } aesthetic <- match.arg(aesthetic) color_levels <- c("light", "mid", "dark") if (diagnostic == "neff_ratio") { diff --git a/tests/testthat/test-mcmc-diagnostics.R b/tests/testthat/test-mcmc-diagnostics.R index 5e95f46be..83cc8af9e 100644 --- a/tests/testthat/test-mcmc-diagnostics.R +++ b/tests/testthat/test-mcmc-diagnostics.R @@ -79,6 +79,13 @@ test_that("'description' & 'rating' columns are correct (#176)", { expect_equal(df$description, expected_descriptions) }) +test_that("diagnostic color helpers handle neff names explicitly", { + expect_no_error(scale_color_diagnostic("neff")) + expect_no_error(scale_fill_diagnostic("neff")) + expect_no_error(diagnostic_color_scale("neff", aesthetic = "color")) + expect_no_error(diagnostic_color_scale("neff_ratio", aesthetic = "color")) +}) + test_that("mcmc_acf & mcmc_acf_bar return a ggplot object", { expect_gg(mcmc_acf(arr, pars = "beta[1]", regex_pars = "x\\:[2,5]")) expect_gg(mcmc_acf_bar(arr, pars = "beta[1]", regex_pars = "x\\:[2,5]")) From 78fb3a7946893860eb9012ed345ca0e268e8dbd2 Mon Sep 17 00:00:00 2001 From: Utkarsh Date: Sat, 14 Mar 2026 00:36:39 +0530 Subject: [PATCH 2/4] Refactor diagnostic alias normalization and strengthen tests --- R/mcmc-diagnostics.R | 18 ++++++++++-------- tests/testthat/test-mcmc-diagnostics.R | 21 +++++++++++++++++++-- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/R/mcmc-diagnostics.R b/R/mcmc-diagnostics.R index defede8fe..18c470ad3 100644 --- a/R/mcmc-diagnostics.R +++ b/R/mcmc-diagnostics.R @@ -425,21 +425,26 @@ diagnostic_points <- function(size = NULL) { # Functions wrapping around scale_color_manual() and scale_fill_manual(), used to # color the intervals by rhat value scale_color_diagnostic <- function(diagnostic = c("rhat", "neff")) { - d <- match.arg(diagnostic) + d <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) diagnostic_color_scale(d, aesthetic = "color") } scale_fill_diagnostic <- function(diagnostic = c("rhat", "neff")) { - d <- match.arg(diagnostic) + d <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) diagnostic_color_scale(d, aesthetic = "fill") } -diagnostic_color_scale <- function(diagnostic = c("rhat", "neff_ratio"), - aesthetic = c("color", "fill")) { +normalize_diagnostic_name <- function(diagnostic) { diagnostic <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) if (diagnostic == "neff") { diagnostic <- "neff_ratio" } + diagnostic +} + +diagnostic_color_scale <- function(diagnostic = c("rhat", "neff_ratio"), + aesthetic = c("color", "fill")) { + diagnostic <- normalize_diagnostic_name(diagnostic) aesthetic <- match.arg(aesthetic) dc <- diagnostic_colors(diagnostic, aesthetic) do.call( @@ -455,10 +460,7 @@ diagnostic_color_scale <- function(diagnostic = c("rhat", "neff_ratio"), diagnostic_colors <- function(diagnostic = c("rhat", "neff_ratio"), aesthetic = c("color", "fill")) { - diagnostic <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) - if (diagnostic == "neff") { - diagnostic <- "neff_ratio" - } + diagnostic <- normalize_diagnostic_name(diagnostic) aesthetic <- match.arg(aesthetic) color_levels <- c("light", "mid", "dark") if (diagnostic == "neff_ratio") { diff --git a/tests/testthat/test-mcmc-diagnostics.R b/tests/testthat/test-mcmc-diagnostics.R index 83cc8af9e..b393d6e8a 100644 --- a/tests/testthat/test-mcmc-diagnostics.R +++ b/tests/testthat/test-mcmc-diagnostics.R @@ -80,10 +80,27 @@ test_that("'description' & 'rating' columns are correct (#176)", { }) test_that("diagnostic color helpers handle neff names explicitly", { + # wrappers accept both aliases expect_no_error(scale_color_diagnostic("neff")) expect_no_error(scale_fill_diagnostic("neff")) - expect_no_error(diagnostic_color_scale("neff", aesthetic = "color")) - expect_no_error(diagnostic_color_scale("neff_ratio", aesthetic = "color")) + expect_no_error(scale_color_diagnostic("neff_ratio")) + expect_no_error(scale_fill_diagnostic("neff_ratio")) + + # aliases map to equivalent scales + color_neff <- scale_color_diagnostic("neff") + color_neff_ratio <- scale_color_diagnostic("neff_ratio") + expect_equal(color_neff$palette(3), color_neff_ratio$palette(3)) + expect_equal(color_neff$labels, color_neff_ratio$labels) + + fill_neff <- scale_fill_diagnostic("neff") + fill_neff_ratio <- scale_fill_diagnostic("neff_ratio") + expect_equal(fill_neff$palette(3), fill_neff_ratio$palette(3)) + expect_equal(fill_neff$labels, fill_neff_ratio$labels) + + base_neff <- diagnostic_color_scale("neff", aesthetic = "color") + base_neff_ratio <- diagnostic_color_scale("neff_ratio", aesthetic = "color") + expect_equal(base_neff$palette(3), base_neff_ratio$palette(3)) + expect_equal(base_neff$labels, base_neff_ratio$labels) }) test_that("mcmc_acf & mcmc_acf_bar return a ggplot object", { From 6c1e4e3863a5694446ee63924f1adef315b250c9 Mon Sep 17 00:00:00 2001 From: Utkarsh Date: Sat, 14 Mar 2026 00:57:35 +0530 Subject: [PATCH 3/4] Simplify diagnostic scale naming to neff_ratio --- NEWS.md | 2 +- R/mcmc-diagnostics.R | 28 +++++++++----------------- tests/testthat/test-mcmc-diagnostics.R | 22 ++------------------ 3 files changed, 13 insertions(+), 39 deletions(-) diff --git a/NEWS.md b/NEWS.md index 64a3c3584..bf6aaef88 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,7 +10,7 @@ * Fixed test in `test-ppc-distributions.R` that incorrectly used `ppc_dens()` instead of `ppd_dens()` when testing PPD functions * New functions `mcmc_dots` and `mcmc_dots_by_chain` for dot plots of MCMC draws by @behramulukir (#402) * Default to `quantiles=100` for all dot plots by @behramulukir (#402) -* Make diagnostic color scale helpers handle `"neff"` and `"neff_ratio"` explicitly, avoiding reliance on partial matching. +* Use `"neff_ratio"` consistently in diagnostic color scale helpers to avoid relying on partial matching of `"neff"`. # bayesplot 1.15.0 diff --git a/R/mcmc-diagnostics.R b/R/mcmc-diagnostics.R index 18c470ad3..2a71b8e00 100644 --- a/R/mcmc-diagnostics.R +++ b/R/mcmc-diagnostics.R @@ -255,8 +255,8 @@ mcmc_neff <- function(ratio, ..., size = NULL) { linetype = 2, linewidth = 0.25) + labs(y = NULL, x = expression(N[eff]/N)) + - scale_fill_diagnostic("neff") + - scale_color_diagnostic("neff") + + scale_fill_diagnostic("neff_ratio") + + scale_color_diagnostic("neff_ratio") + scale_x_continuous( breaks = breaks, # as.character truncates trailing zeroes, while ggplot default does not @@ -287,8 +287,8 @@ mcmc_neff_hist <- function(ratio, ..., binwidth = NULL, bins = NULL, breaks = NU binwidth = binwidth, bins = bins, breaks = breaks) + - scale_color_diagnostic("neff") + - scale_fill_diagnostic("neff") + + scale_color_diagnostic("neff_ratio") + + scale_fill_diagnostic("neff_ratio") + labs(x = expression(N[eff]/N), y = NULL) + dont_expand_y_axis(c(0.005, 0)) + yaxis_title(FALSE) + @@ -424,27 +424,19 @@ diagnostic_points <- function(size = NULL) { # Functions wrapping around scale_color_manual() and scale_fill_manual(), used to # color the intervals by rhat value -scale_color_diagnostic <- function(diagnostic = c("rhat", "neff")) { - d <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) +scale_color_diagnostic <- function(diagnostic = c("rhat", "neff_ratio")) { + d <- match.arg(diagnostic) diagnostic_color_scale(d, aesthetic = "color") } -scale_fill_diagnostic <- function(diagnostic = c("rhat", "neff")) { - d <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) +scale_fill_diagnostic <- function(diagnostic = c("rhat", "neff_ratio")) { + d <- match.arg(diagnostic) diagnostic_color_scale(d, aesthetic = "fill") } -normalize_diagnostic_name <- function(diagnostic) { - diagnostic <- match.arg(diagnostic, choices = c("rhat", "neff", "neff_ratio")) - if (diagnostic == "neff") { - diagnostic <- "neff_ratio" - } - diagnostic -} - diagnostic_color_scale <- function(diagnostic = c("rhat", "neff_ratio"), aesthetic = c("color", "fill")) { - diagnostic <- normalize_diagnostic_name(diagnostic) + diagnostic <- match.arg(diagnostic) aesthetic <- match.arg(aesthetic) dc <- diagnostic_colors(diagnostic, aesthetic) do.call( @@ -460,7 +452,7 @@ diagnostic_color_scale <- function(diagnostic = c("rhat", "neff_ratio"), diagnostic_colors <- function(diagnostic = c("rhat", "neff_ratio"), aesthetic = c("color", "fill")) { - diagnostic <- normalize_diagnostic_name(diagnostic) + diagnostic <- match.arg(diagnostic) aesthetic <- match.arg(aesthetic) color_levels <- c("light", "mid", "dark") if (diagnostic == "neff_ratio") { diff --git a/tests/testthat/test-mcmc-diagnostics.R b/tests/testthat/test-mcmc-diagnostics.R index b393d6e8a..c57d9eaef 100644 --- a/tests/testthat/test-mcmc-diagnostics.R +++ b/tests/testthat/test-mcmc-diagnostics.R @@ -79,28 +79,10 @@ test_that("'description' & 'rating' columns are correct (#176)", { expect_equal(df$description, expected_descriptions) }) -test_that("diagnostic color helpers handle neff names explicitly", { - # wrappers accept both aliases - expect_no_error(scale_color_diagnostic("neff")) - expect_no_error(scale_fill_diagnostic("neff")) +test_that("diagnostic color helpers use neff_ratio consistently", { expect_no_error(scale_color_diagnostic("neff_ratio")) expect_no_error(scale_fill_diagnostic("neff_ratio")) - - # aliases map to equivalent scales - color_neff <- scale_color_diagnostic("neff") - color_neff_ratio <- scale_color_diagnostic("neff_ratio") - expect_equal(color_neff$palette(3), color_neff_ratio$palette(3)) - expect_equal(color_neff$labels, color_neff_ratio$labels) - - fill_neff <- scale_fill_diagnostic("neff") - fill_neff_ratio <- scale_fill_diagnostic("neff_ratio") - expect_equal(fill_neff$palette(3), fill_neff_ratio$palette(3)) - expect_equal(fill_neff$labels, fill_neff_ratio$labels) - - base_neff <- diagnostic_color_scale("neff", aesthetic = "color") - base_neff_ratio <- diagnostic_color_scale("neff_ratio", aesthetic = "color") - expect_equal(base_neff$palette(3), base_neff_ratio$palette(3)) - expect_equal(base_neff$labels, base_neff_ratio$labels) + expect_no_error(diagnostic_color_scale("neff_ratio", aesthetic = "color")) }) test_that("mcmc_acf & mcmc_acf_bar return a ggplot object", { From 4d335f2a2ed017b1b22bfd1202653f373efa206e Mon Sep 17 00:00:00 2001 From: jgabry Date: Fri, 13 Mar 2026 15:22:23 -0600 Subject: [PATCH 4/4] Remove unneeded tests --- tests/testthat/test-mcmc-diagnostics.R | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/testthat/test-mcmc-diagnostics.R b/tests/testthat/test-mcmc-diagnostics.R index c57d9eaef..5e95f46be 100644 --- a/tests/testthat/test-mcmc-diagnostics.R +++ b/tests/testthat/test-mcmc-diagnostics.R @@ -79,12 +79,6 @@ test_that("'description' & 'rating' columns are correct (#176)", { expect_equal(df$description, expected_descriptions) }) -test_that("diagnostic color helpers use neff_ratio consistently", { - expect_no_error(scale_color_diagnostic("neff_ratio")) - expect_no_error(scale_fill_diagnostic("neff_ratio")) - expect_no_error(diagnostic_color_scale("neff_ratio", aesthetic = "color")) -}) - test_that("mcmc_acf & mcmc_acf_bar return a ggplot object", { expect_gg(mcmc_acf(arr, pars = "beta[1]", regex_pars = "x\\:[2,5]")) expect_gg(mcmc_acf_bar(arr, pars = "beta[1]", regex_pars = "x\\:[2,5]"))