tidyverse / ggplot2

An implementation of the Grammar of Graphics in R
https://ggplot2.tidyverse.org
Other
6.5k stars 2.02k forks source link

geom_area with negative y not working in ggplot2 3.2.0 #3390

Closed vadimus202 closed 5 years ago

vadimus202 commented 5 years ago

In ggplot2 3.1.1 the area under the curve handles negative values

library(ggplot2)

set.seed(115)

ggplot(
    data.frame(
        x = 1:100,
        y = cumsum(rnorm(n = 100, mean = 0, sd = 3))
    ),
    aes(x, y)) +
    geom_area(fill = "pink", alpha = 0.5) +
    geom_path()

image

In ggplot2 3.2.0 something goes wrong:

library(ggplot2)

set.seed(115)

ggplot(
    data.frame(
        x = 1:100,
        y = cumsum(rnorm(n = 100, mean = 0, sd = 3))
    ),
    aes(x, y)) +
    geom_area(fill = "pink", alpha = 0.5) +
    geom_path()

image

yutannihilation commented 5 years ago

A simpler version of code (Sorry, I can't render the reprex right now...). Not sure what's happening here, but it seems something went wrong...

library(ggplot2)

d <- data.frame(
  x = 1:6,
  y = c(1, 2, 2, 1, 0, -1)
)

ggplot(d, aes(x, y)) +
  geom_area(fill = "pink", alpha = 0.5) +
  geom_point()

ggplot(d, aes(x, y - 1)) +
  geom_area(fill = "pink", alpha = 0.5) +
  geom_point()
clauswilke commented 5 years ago

Rendered reprex below.

@thomasp85 Is this somehow related to the rewritten polygon rendering code? We may have to consider a hotfix release, since this is a serious regression.

library(ggplot2)

d <- data.frame(
  x = 1:6,
  y = c(1, 2, 2, 1, 0, -1)
)

ggplot(d, aes(x, y)) +
  geom_area(fill = "pink", alpha = 0.5) +
  geom_point()


ggplot(d, aes(x, y - 1)) +
  geom_area(fill = "pink", alpha = 0.5) +
  geom_point()

Created on 2019-07-03 by the reprex package (v0.3.0)

thomasp85 commented 5 years ago

I’ll look into it.

thomasp85 commented 5 years ago

This is due to GeomArea/GeomRibbon now sorting their data in setup_data instead of in draw_panel. PositionStack changes the ordering of the data and that is no longer resolved as it comes after setup_data.

There are two ways around this.

1) Make sure PositionStack doesn't change the ordering of data 2) Make a secondary sort in draw_panel()

I generally prefer 1) as it is how I think positions should behave (i.e. not fiddle with data ordering), but it may have unforeseen consequences 🤷‍♂️

I agree this should be fixed in a hot fix (and we really should have some tests to catch this) cc @hadley

hadley commented 5 years ago

1) sounds reasonable. And it shouldn't be too bad to do a patch release for this issue. Do we want to wait a week in case any other problems come in?

thomasp85 commented 5 years ago

Let's wait a week before we start release process

karawoo commented 5 years ago

If I recall correctly, the ordering in PositionStack exists to fix my first ever bug report (#1552) 🙂. So it is there for a reason, but it's possible that reason has been obviated by other changes since. I can try to look into it this weekend.

karawoo commented 5 years ago

I've opened a PR that will address this, but in the meantime geom_ribbon() still works:

library(ggplot2)

set.seed(115)

ggplot(
  data.frame(
    x = 1:100,
    y = cumsum(rnorm(n = 100, mean = 0, sd = 3))
  ),
  aes(x, y)) +
  geom_ribbon(aes(ymax = y, ymin = 0), fill = "pink", alpha = 0.5) +
  geom_path()

Created on 2019-07-17 by the reprex package (v0.3.0)

yutannihilation commented 5 years ago

@thomasp85 @hadley

Let's wait a week before we start release process

Some weeks have passed. Do we start the process? While this issue should be hot-fixed, this might not be as serious as we thought at first in terms of the number of users affected, considering the fact there's no other issues filed than this one. So, if you are very busy (sorry, I don't think I can review #3401...), I think it's okay not to hurry so much.

But, I'm a bit worrying about that the CRAN check is all red probably due to #3394 (, which seems already fixed by @paleolimbot). Do you get any serious warnings about this from CRAN people?

https://cran.rstudio.com/web/checks/check_results_ggplot2.html

thomasp85 commented 5 years ago

I’m currently on vacation (this week and next). I’ll begin release once I’m back

yutannihilation commented 5 years ago

OK. Have a nice vacation!

lock[bot] commented 4 years ago

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/