Skip to content

Make annotate_logticks coord_flip-aware #4431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thomasp85 merged 2 commits into from
Apr 27, 2021
Merged

Make annotate_logticks coord_flip-aware #4431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a46b3a
Make annotate_logticks coord_flip-aware
thomasp85 Apr 19, 2021
315386a
Merge branch 'master' into issue-3954-annotate-logticks-flip
thomasp85 Apr 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# ggplot2 (development version)


* Fix bug in `annotate_logticks()` that would cause an error when used together
with `coord_flip()` (@thomasp85, #3954)

* Fix a bug in `guide_bins()` where keys would disappear if the guide was
reversed (@thomasp85, #4210)

Expand Down
9 changes: 5 additions & 4 deletions R/annotation-logticks.r
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,15 @@ GeomLogticks <- ggproto("GeomLogticks", Geom,
mid = unit(0.2, "cm"), long = unit(0.3, "cm"))
{
ticks <- list()
flipped <- inherits(coord, "CoordFlip")
Copy link
Member

@yutannihilation yutannihilation Apr 20, 2021

Choose a reason for hiding this comment

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

This feels a bit too specific to this problem. Are there any other way to check if the coord is flipped? (I guess the answer is no, but let me just confirm...)

Copy link
Member Author

@thomasp85 thomasp85 Apr 26, 2021

Choose a reason for hiding this comment

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

The answer is no :-)... We have these annoying specific logic pertaining to coord_flip around the code base and there is not really much to do about it at this time

We can discuss a more throughout fix at a later stage but that is beyond this fix

Copy link
Member

@yutannihilation yutannihilation Apr 26, 2021

Choose a reason for hiding this comment

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

I see, thanks for clarifying.

x_name <- if (flipped) "y" else "x"
y_name <- if (flipped) "x" else "y"

# Convert these units to numbers so that they can be put in data frames
short <- convertUnit(short, "cm", valueOnly = TRUE)
mid <- convertUnit(mid, "cm", valueOnly = TRUE)
long <- convertUnit(long, "cm", valueOnly = TRUE)


if (grepl("[b|t]", sides)) {

# Get positions of x tick marks
Expand All @@ -149,7 +151,7 @@ GeomLogticks <- ggproto("GeomLogticks", Geom,
if (scaled)
xticks$value <- log(xticks$value, base)

names(xticks)[names(xticks) == "value"] <- "x" # Rename to 'x' for coordinates$transform
names(xticks)[names(xticks) == "value"] <- x_name # Rename to 'x' for coordinates$transform
xticks <- coord$transform(xticks, panel_params)
xticks = xticks[xticks$x <= 1 & xticks$x >= 0,]

Expand All @@ -173,7 +175,6 @@ GeomLogticks <- ggproto("GeomLogticks", Geom,
}
}


if (grepl("[l|r]", sides)) {
yticks <- calc_logticks(
base = base,
Expand All @@ -188,7 +189,7 @@ GeomLogticks <- ggproto("GeomLogticks", Geom,
if (scaled)
yticks$value <- log(yticks$value, base)

names(yticks)[names(yticks) == "value"] <- "y" # Rename to 'y' for coordinates$transform
names(yticks)[names(yticks) == "value"] <- y_name # Rename to 'y' for coordinates$transform
yticks <- coord$transform(yticks, panel_params)
yticks = yticks[yticks$y <= 1 & yticks$y >= 0,]

Expand Down