Skip to content

Fix mcmc_areas_ridges() overlay breaking with + scale_y_discrete()#432

Open
utkarshpawade wants to merge 2 commits intostan-dev:masterfrom
utkarshpawade:issue-287-mcmc-areas-ridges
Open

Fix mcmc_areas_ridges() overlay breaking with + scale_y_discrete()#432
utkarshpawade wants to merge 2 commits intostan-dev:masterfrom
utkarshpawade:issue-287-mcmc-areas-ridges

Conversation

@utkarshpawade
Copy link
Contributor

Fixes #287

mcmc_areas_ridges() used scale_y_discrete(limits = ...) internally to set the y-axis draw order. When users added their own + scale_y_discrete(...) (eg to rename labels), ggplot2 replaced the entire scale (including the limits) which flipped the draw order and broke ridgeline masking.

This fix encodes the draw order directly in the factor levels of
data$parameter (reversed so the first parameter appears on top), instead of relying on limits =. User-added scale_y_discrete() calls can now change labels without affecting the overlay order.

Changes:

  • mcmc-intervals.R: reverse factor levels before building layers,
    remove limits = from scale_y_discrete(), update par_draw_order
  • test-mcmc-intervals.R: add unit tests and vdiffr snapshot for the
    previously failing case

Copilot AI review requested due to automatic review settings March 10, 2026 09:08
@utkarshpawade utkarshpawade changed the title Fix mcmc_areas_ridges() overlay breaking with + scale_y_discrete() (#287) Fix mcmc_areas_ridges() overlay breaking with + scale_y_discrete() Mar 10, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where mcmc_areas_ridges() would break when users appended + scale_y_discrete() to the returned plot. Previously, the draw order was enforced via limits = inside an internal scale_y_discrete() call; ggplot2 silently replaced this when a user supplied their own scale, discarding the limits = and flipping the ridgeline draw order.

Changes:

  • Draw order is now encoded directly in the factor levels of data$parameter (reversed), removing the need for limits = in scale_y_discrete()
  • par_draw_order updated to rev(levels(...)) to match the new factor encoding
  • Unit tests and a vdiffr snapshot added for the regression case

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
R/mcmc-intervals.R Core fix: reverses factor levels on data$parameter and drops limits = from scale_y_discrete()
tests/testthat/test-mcmc-intervals.R Adds unit test and vdiffr snapshot for the previously broken scale_y_discrete overlay case
NEWS.md Documents the bug fix in the changelog

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.63%. Comparing base (306c92e) to head (e1008c9).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
- Coverage   98.66%   98.63%   -0.04%     
==========================================
  Files          35       35              
  Lines        5860     5862       +2     
==========================================
  Hits         5782     5782              
- Misses         78       80       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jgabry
Copy link
Member

jgabry commented Mar 10, 2026

Thanks for the PR! I'm not sure this is working correctly. This does fix the overlay issue when using scale_y_discrete, however when I try the examples from #287 using this branch it seems like the y axis labels are reordered but the actual density plots aren't reordered. Is that right? Do you see the same thing when you try those examples or is it working correctly for you?

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.

mcmc_areas_ridges don't overlay when using scale_y_discrete

4 participants