Skip to content

flip = TRUE: flip the drawn geometry of "h" and the step types - #678

Open
haomeng797-ship-it wants to merge 1 commit into
grantmcdermott:mainfrom
haomeng797-ship-it:flip-line-types
Open

flip = TRUE: flip the drawn geometry of "h" and the step types#678
haomeng797-ship-it wants to merge 1 commit into
grantmcdermott:mainfrom
haomeng797-ship-it:flip-line-types

Conversation

@haomeng797-ship-it

Copy link
Copy Markdown
Contributor

Fixes #675.

flip_datapoints() swaps the coordinates, but lines() draws some single-letter types in a fixed orientation regardless of the data: type = "h" always drops vertically, and the step types commit to which coordinate moves first. So the axes flipped while the marks kept their unflipped geometry, which is the middle panel in the issue.

Changes in draw_lines(), all behind isTRUE(flip) so nothing else is touched:

  • "h" now draws explicit horizontal segments from the baseline (0, or the left edge under log = "x", matching how the base engine anchors its vertical drops).
  • "s" and "S" swap under flip: reflecting a step path across the diagonal turns a move-then-rise step into a rise-then-move one, so the flipped rendering of "s" is lines(type = "S") on the swapped data, and vice versa. Both were silently drawing mirrored steps before.
  • "l", "b", "o", "c" are flip-invariant and unchanged.

On @zeileis's question about a type = "H" spelling: this PR only makes flip behave, so the axes stay swapped (value axis horizontal). A standalone "H" that keeps x as x would be a separate, additive decision, and nothing here gets in its way. Happy to do that as a follow-up if you want it.

Tests: rather than snapshots, the new tests in test-flip.R read the SVG output and assert the geometry directly (flipped "h" emits one horizontal <line> per point, unflipped stays vertical, and the first drawn step segment of flipped "s"/"S" moves in the right direction). They run on any OS, so they also exercise the fix on macOS where the snapshot suite skips.

lines(type = 'h') always drops vertically and the step types fix which
coordinate moves first, so swapping the data alone leaves the marks in
the unflipped orientation. Draw flipped 'h' as horizontal segments to
the baseline and exchange 's'/'S' under flip. Geometry checked from the
SVG output directly so the new tests run on any OS.
@grantmcdermott

Copy link
Copy Markdown
Owner

This is great, thanks @haomeng797-ship-it.

I'll let @zeileis test it, since he was the OP.

RE: tests. Your inversion test is clever... But I really would prefer dedicated snapshots. We do ship a dedicated .devcontainer for running and creating these Linux-bases snapshots on MacOS. I use finch on my own Mac for this:

brew install --cask finch
finch vm init
finch vm start

Then, just open up VS Code in the directory of your locally cloned version of this repo. It should prompt you to "Reopen in container" (say yes). To run the tests, open up a terminal in VS Code (cmd + j) and run:

make testall

You'll probably get one or two false positive failures. But any new snapshots you generate should be good to go. More details here: https://github.com/grantmcdermott/tinyplot/blob/main/CLAUDE.md#testing--ci

Again, not a train smash if you can't do this. But useful to know, especially if you plan to continue making (these very useful!) PRs.

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.

Support type = "h" with flip = TRUE

2 participants