diff --git a/NEWS.md b/NEWS.md index 25c323e3..959a84e0 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) +* Fix `mcmc_areas_ridges()` overlay breaking when `+ scale_y_discrete()` is added (#287) # bayesplot 1.15.0 diff --git a/R/mcmc-intervals.R b/R/mcmc-intervals.R index 13687846..1e5d4943 100644 --- a/R/mcmc-intervals.R +++ b/R/mcmc-intervals.R @@ -485,6 +485,14 @@ mcmc_areas_ridges <- function(x, bw = bw, adjust = adjust, kernel = kernel, n_dens = n_dens, bounds = bounds) + # Encode the intended display order (first parameter at top, last at bottom) + # directly in the factor levels. This way the default scale_y_discrete does + # not require an explicit `limits =` argument to produce the correct ordering, + # so the plot remains correct when users add their own scale_y_discrete (e.g. + # to relabel or reorder the y-axis) — which would otherwise silently discard + # the `limits =` argument and break the layer masking draw order. + data$parameter <- factor(data$parameter, levels = rev(levels(data$parameter))) + datas <- data %>% split(data$interval) @@ -524,7 +532,11 @@ mcmc_areas_ridges <- function(x, # Draw each ridgeline from top the bottom layer_list_inner <- list() - par_draw_order <- levels(unique(data$parameter)) + # After reversing the factor levels above, the first level is the bottom-most + # parameter and the last is the top-most. `par_draw_order` must go from the + # top-most to the bottom-most so that each successive layer correctly masks + # the outer ridgelines of the parameters below it. + par_draw_order <- rev(levels(unique(data$parameter))) bg <- bayesplot_theme_get()[["panel.background"]][["fill"]] %||% "white" for (par_num in seq_along(unique(data$parameter))) { @@ -557,11 +569,16 @@ mcmc_areas_ridges <- function(x, ggplot(datas$outer) + aes(x = .data$x, y = .data$parameter) + layer_outer + - scale_y_discrete(limits = unique(rev(data$parameter)), - expand = expansion( - add = c(0, 1.4 + 1/(2 * nlevels(data$parameter))), - mult = c(0.05, 1/(2 * nlevels(data$parameter))) - )) + + # No `limits =` needed: factor levels already encode the correct display + # order so that the default scale_y_discrete produces the right result. + # Omitting `limits =` means a user-supplied scale_y_discrete (e.g. to + # rename labels) will not silently break the overlay. + scale_y_discrete( + expand = expansion( + add = c(0, 1.4 + 1 / (2 * nlevels(data$parameter))), + mult = c(0.05, 1 / (2 * nlevels(data$parameter))) + ) + ) + layer_list_inner + layer_vertical_line + scale_fill_identity() + diff --git a/tests/testthat/_snaps/mcmc-intervals/mcmc-areas-ridges-scale-y-discrete-labels.svg b/tests/testthat/_snaps/mcmc-intervals/mcmc-areas-ridges-scale-y-discrete-labels.svg new file mode 100644 index 00000000..24495bca --- /dev/null +++ b/tests/testthat/_snaps/mcmc-intervals/mcmc-areas-ridges-scale-y-discrete-labels.svg @@ -0,0 +1,68 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +z +y +x + + + + + + + +-2 +0 +2 +mcmc_areas_ridges (scale_y_discrete labels) + + diff --git a/tests/testthat/test-mcmc-intervals.R b/tests/testthat/test-mcmc-intervals.R index 882109bd..0e670452 100644 --- a/tests/testthat/test-mcmc-intervals.R +++ b/tests/testthat/test-mcmc-intervals.R @@ -78,6 +78,26 @@ test_that("mcmc_areas_ridges returns a ggplot object", { expect_gg(mcmc_areas_ridges(dframe1)) }) +test_that("mcmc_areas_ridges overlay is not broken by scale_y_discrete (#287)", { + # Adding scale_y_discrete (e.g. to rename labels) used to silently replace + # the internal `limits =` that encodes the y-axis draw order, causing the + # ridgeline masking to break. + pars3 <- c("V1", "V2", "V3") + p_base <- mcmc_areas_ridges(vdiff_dframe, pars = pars3) + + # The plot still builds cleanly after adding a user-supplied scale_y_discrete + expect_gg(p_base + ggplot2::scale_y_discrete(labels = c("A", "B", "C"))) + expect_gg(p_base + ggplot2::scale_y_discrete(labels = pars3)) + + # mcmc_areas_ridges() reverses factor levels internally so that the first + # parameter sits at the top of the plot without relying on `limits =`. + # Check the built plot data to confirm the reversal. + plot_data <- ggplot2::ggplot_build(p_base) + y_limits <- plot_data$layout$panel_scales_y[[1]]$get_limits() + expect_equal(y_limits[1], "V3") + expect_equal(y_limits[length(y_limits)], "V1") +}) + test_that("mcmc_intervals/areas with rhat", { r <- runif(ncol(mat), 0.9, 1.3) rbad <- c(NA, r[-1]) @@ -254,4 +274,9 @@ test_that("mcmc_areas_ridges renders correctly", { p_size <- mcmc_areas_ridges(vdiff_dframe, border_size = 2) vdiffr::expect_doppelganger("mcmc_areas_ridges (size)", p_size) + + # Regression test for #287: overlay must remain correct after scale_y_discrete + p_labels <- mcmc_areas_ridges(vdiff_dframe, pars = c("V1", "V2", "V3")) + + ggplot2::scale_y_discrete(labels = c("z", "y", "x")) + vdiffr::expect_doppelganger("mcmc_areas_ridges (scale_y_discrete labels)", p_labels) })