Skip to content

Conversation

@clauswilke
Copy link
Member

I think the way to fix this issue is to give geom_smooth() a first-class se parameter that is considered when drawing the panel. This allows the geom and the legend drawing code to always be in synch. See resulting behavior in the examples below.

One note: Currently one regression test fails because the geom_smooth() and draw_group() functions have different default arguments and the test checks for that. I think the test is wrong in this particular case, but I wanted to bring this up for discussion. Relevant lines in the code, with explanation for why it should be this way:
https://github.com/clauswilke/ggplot2/blob/ed016144c97cf67424498453a2f4e06925fdba92/R/geom-smooth.r#L119-L126

Examples of geom_smooth() with stat_summary():

library(ggplot2)
ggplot(mtcars, aes(cyl, mpg, fill = factor(am))) +
  geom_smooth(stat = "summary")
#> No summary function supplied, defaulting to `mean_se()

ggplot(mtcars, aes(cyl, mpg, fill = factor(am))) +
  geom_smooth(stat = "summary", se = FALSE)
#> No summary function supplied, defaulting to `mean_se()

# stat_summary doesn't have an `se` parameter, hence
# default is `se = FALSE`
ggplot(mtcars, aes(cyl, mpg, fill = factor(am))) +
  stat_summary(geom = "smooth")
#> No summary function supplied, defaulting to `mean_se()

ggplot(mtcars, aes(cyl, mpg, fill = factor(am))) +
  stat_summary(geom = "smooth", se = TRUE)
#> No summary function supplied, defaulting to `mean_se()

And, for completeness, all combinations of geom_smooth() / stat_smooth() with se = TRUE / se = FALSE work as expected:

library(ggplot2)
ggplot(iris, aes(Sepal.Length, Sepal.Width, color = Species, fill = Species)) +
  geom_smooth()
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'

ggplot(iris, aes(Sepal.Length, Sepal.Width, color = Species, fill = Species)) +
  geom_smooth(se = FALSE)
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'

ggplot(iris, aes(Sepal.Length, Sepal.Width, color = Species, fill = Species)) +
  stat_smooth()
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'

ggplot(iris, aes(Sepal.Length, Sepal.Width, color = Species, fill = Species)) +
  stat_smooth(se = FALSE)
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'

Created on 2018-05-10 by the reprex package (v0.2.0).

@hadley
Copy link
Member

hadley commented May 10, 2018

I think this approach is reasonable; the way parameters works is all a little hacky, so you should feel free to disable the test for this case (IIRC there are already a few other geoms where it's disabled)

@clauswilke
Copy link
Member Author

I've disabled the consistency check for geom_smooth(). (You were correct, there was already a list of such geoms.) I have added two simple visual tests, since there weren't any for geom_smooth() or stat_smooth() yet.

@hadley hadley merged commit 8e365a4 into tidyverse:master May 10, 2018
@hadley
Copy link
Member

hadley commented May 10, 2018

Thanks!

@lock
Copy link

lock bot commented Nov 17, 2018

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/

@lock lock bot locked and limited conversation to collaborators Nov 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants