Conversation
ggplot2's Guides$merge() breaks ties between same-order guides using a content hash (rlang::hash()), which has no relation to aesthetic declaration order. A recent rlang release changed that hash, flipping the tie-break and silently reordering combined legend titles (e.g. "factor(vs)<br />factor(cyl)" became "factor(cyl)<br />factor(vs)"). Legend entries themselves are named independently, always in aesthetic declaration order (layers2traces.R's discreteScales), so title order and entry order could now disagree. Reorder guides in get_gdefs_ggproto() by (explicit guide order, aesthetic declaration order) so the title always agrees with the entries it labels, regardless of upstream hashing changes.
Silences the R CMD check NOTE about deprecated special names in structure() calls.
R CMD check & CI passing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
CRAN and CI started failing on
test-ggplot-legend.Rwith no code changes on our side:Root cause: when a plot has two separate discrete legends (e.g.
color+shape) that ggplot2 can't merge into one, ggplot2 orders them for displayusing a content hash (
rlang::hash()) as a tie-break whenever neither guidehas an explicit
order=. A recentrlangrelease changed how that hash iscomputed ("all hash values will now be different" - rlang 1.3.0 NEWS), which
silently flipped which guide sorts first - in real
ggplot2::ggplotGrob()renders too, not just plotly's output. Nothing in this package or in user
plot code changed; a transitive dependency bump reordered something ggplot2
never actually guaranteed the order of.
That would just be cosmetic, except plotly builds the combined legend title
(
R/ggplotly.R) from that same hash-ordered guide list, while the legendentries underneath it (the actual traces/markers) are named independently,
always in the order aesthetics were declared in
aes()(
layers2traces.R'sdiscreteScales). So the rlang bump made the titleorder and the entry order disagree with each other, which is the actual bug
worth fixing here (not just "match whatever ggplot2 happens to do this
week").
What changed
get_gdefs_ggproto()now re-sorts guides afterguides$merge()by(explicit guide order, aesthetic declaration order)instead of leavingggplot2's hash tie-break in place. This:
regardless of future hashing/rlang/ggplot2 changes
guide_legend(order = ...)when the user setsone (see the "legend can be manipulated via guides()" test, which
deliberately declares
shapebeforecolorinaes()but orders them theopposite way via
order=)Known gap (not fixed here, on purpose): the pre-ggplot2-3.5.0 legacy guide
code path (
get_gdefs()) has the identical latent ordering issue, but it'suntestable in any currently-installed ggplot2 and effectively dead code, so
it was left alone.
Follow-up needed:
tests/testthat/_snaps/ggplot-legend/scatter-legend.svgstill encodes the old (regressed) title order and needs to be regenerated
via the visual-test Docker flow before the macOS/VISUAL_TESTS CI leg will
pass again.