Skip to content

Conversation

@yutannihilation
Copy link
Member

Fix #4023

The current implementation squashes the upper and lower lines together and tweaks the lower one's id on the assumption that the upper and the lower has the same number of rows as the original data. But, it's not true when the Coord is non-cartesian.

ggplot2/R/geom-ribbon.r

Lines 131 to 135 in 4826838

positions <- new_data_frame(list(
x = c(data$x, rev(data$x)),
y = c(data$ymax, rev(data$ymin)),
id = c(ids, rev(ids))
))

munched <- coord_munch(coord, positions, panel_params)

ggplot2/R/geom-ribbon.r

Lines 158 to 164 in 4826838

# increment the IDs of the lower line
munched_lines$id <- switch(outline.type,
both = munched_lines$id + rep(c(0, max(ids, na.rm = TRUE)), each = length(ids)),
upper = munched_lines$id + rep(c(0, NA), each = length(ids)),
lower = munched_lines$id + rep(c(NA, 0), each = length(ids)),
abort(glue("invalid outline.type: {outline.type}"))
)

This PR fixes this issue by processing the upper and the lower separately.

devtools::load_all("~/repo/ggplot2/")
#> Loading ggplot2

d <- base::data.frame(x = c(1, 2), y = c(3, 4))

ggplot(d) +
  geom_area(aes(x, y), colour = "black", fill = "pink") +
  coord_polar()

Created on 2020-05-24 by the reprex package (v0.3.0)

@yutannihilation
Copy link
Member Author

@thomasp85
If there's still time left, can we include this in v3.3.1 since this is a kind of regression bug introduced in v3.3.0? If not, I think it's ok to wait for another release, though.

@clauswilke
Copy link
Member

For what it's worth, I think it would be good to merge this and #4026 before the release.

@thomasp85
Copy link
Member

I don’t see any problem merging both of these

@yutannihilation
Copy link
Member Author

Thanks, then I'm merging this.

@yutannihilation yutannihilation merged commit 323af07 into tidyverse:master May 25, 2020
@yutannihilation yutannihilation deleted the fix/issue-4023-geom-ribbon-on-polar-coord branch May 25, 2020 08:01
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.

munched_lines warning from geom_area()

3 participants