Repository navigation
Fix coord_trans_xy() for layers with a single x or y value - #122
Open
willgearty wants to merge 1 commit into
Open
willgearty wants to merge 1 commit into
willgearty wants to merge 1 commit into
Conversation
The axis tick label and axis line hacks in CoordTransXY$transform() identified axis data by shape alone, so layers whose points all share an x or y value (e.g., geom_hline(), geom_vline(), a single point or label) were treated as axes and dropped or misplaced. Only apply those hacks to data without a group column, which all layer data has. Also update the docs: geom_rect() and similar geoms already work, since ggplot2 converts them to polygons for non-linear coordinate systems. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01A7zDoJJQSEuNmyB6xR3njB
This branch has not been deployed
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.
CoordTransXY$transform()recognizes axis tick labels and axis lines by the shape of their data: allx/yequal toInf/-Inf, or a single uniquexoryvalue. Layer data with that shape was treated as an axis too, so withcoord_trans_xy():geom_hline()andgeom_vline()weren't drawn,geom_line()or a singleannotate("text")wasn't drawn,Axis guide data reaching
transform()never has agroupcolumn (tick labels havex,y,.value,.label; axis lines onlyx,y), while ggplot2 gives all layer data agroupcolumn. So:groupcolumn. Layer data is always transformed normally. The hacks themselves are unchanged, and their inline comments moved inside each branch to keep the lines short.@detailscaveat.geom_rect()and similar geoms (geom_tile(),geom_col(),annotate("rect")with infinite limits) already work, because ggplot2 converts them to polygons for non-linear coordinate systems.geom_raster()needs ggplot2 >= 4.0.0, which falls back to drawing rectangles; ggplot2 3.5.x refusesgeom_raster()with any non-Cartesian coord. Also removes the stale# TODO: transform corners for geom_rect?comment.Tests (
test-coord_trans_xy.R): a plot withgeom_hline(),geom_vline(), a flatgeom_line(), a single point and a single text label undershear(2, 0)+rotate(-pi / 3). For each layer, the drawn coordinates are back-transformed and checked against the data values (e.g.y == 1along the whole hline). It also adds atheme_void()snapshot, for both ggplot2 4.x and pre-4.0, so the snapshot doesn't depend on the pre-4.0 axis fix in #117. Without the code change, all five position checks fail.Locally, all 23
coord_trans_xytests pass with ggplot2 4.0.3, including the existing capped-axis, reflect and reverse-scale snapshots, so axis rendering is unchanged. With ggplot2 3.5.2 the new checks pass and the only failures are the outdated pre-4.0coord_trans_xysnapshots that #117 fixes, the same as onmain.This relies on ggplot2 always giving layer data a
groupcolumn. That held in both versions I tested (3.5.2 and 4.0.3) and has been ggplot2's behavior for a long time, but it isn't formally documented.This PR edits the same
transform()branches as #117 (which changes theInfsign checks), and the top ofNEWS.mdlike #116, #117, #120 and #121. Whichever merges later will need a small merge; the two changes combine without conflict in logic.TODO:
AI disclosure: Claude identified the problem and drafted the fix.
🤖 Generated with Claude Code
https://claude.ai/code/session_01A7zDoJJQSEuNmyB6xR3njB
Generated by Claude Code