Skip to content
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.

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

stat_function() produces incorrect warning #3611

Closed
clauswilke opened this issue Nov 4, 2019 · 10 comments · Fixed by #3982
Closed

stat_function() produces incorrect warning #3611

clauswilke opened this issue Nov 4, 2019 · 10 comments · Fixed by #3982

Comments

@clauswilke
Copy link
Member

clauswilke commented Nov 4, 2019

stat_function() generates a warning whenever a mapping is provided, but then it uses the mapping as normal.

These are the relevant lines in the code, and a reprex follows below.

# Warn if supplied mapping and/or data is going to be overwritten
if (!is.null(mapping)) {
warning("`mapping` is not used by stat_function()", call. = FALSE)
}

library(ggplot2)

ggplot(data.frame(x = 0), aes(x = x)) +
  stat_function(aes(color = "linear"), fun = function(x) x) +
  stat_function(aes(color = "quadratic"), fun = function(x) x*x) +
  xlim(-5, 5)
#> Warning: `mapping` is not used by stat_function()

#> Warning: `mapping` is not used by stat_function()

Created on 2019-11-04 by the reprex package (v0.3.0)

I'm not sure whether there are cases where the warning is useful. I would propose to delete it. It's probably too complicated to try to find out whether the mapping is meaningful or not in the given context.

@thomasp85
Copy link
Member

thomasp85 commented Nov 4, 2019

Could it be tweaked to say that positional mapping is ignored?

@clauswilke
Copy link
Member Author

clauswilke commented Nov 4, 2019

I guess we could inspect the mapping and create a warning if positional aesthetics are used.

@clauswilke
Copy link
Member Author

clauswilke commented Nov 4, 2019

Pinging @paleolimbot as he suggested the warning in the first place. Any thoughts on this from you? In the linked discussion, I see mentioning of the group aesthetic, which also may not work as expected. Should we issue a warning if x, y, or group are used?

@paleolimbot
Copy link
Member

paleolimbot commented Nov 5, 2019

Whoops, that was me! Using stat_function() with multiple layers + a manually specified aesthetic should have occurred to me, but didn't...I also didn't know that missing grouping aesthetics from the output of compute_panel() are added back in in compute_panel().

I seem to remember that the issues are caused when stat_function() draws multiple copies of itself because of unknowingly specified groups. Specifying x and y doesn't cause errors but also don't do anything (unless one of them is discrete). Maybe we could (1) warn for mapped x or y in the constructor and/or (2) do a simple override of compute_panel() that just calls Stat$compute_panel() and makes sure that there is exactly one group?

@jan-glx
Copy link
Contributor

jan-glx commented Feb 11, 2020

A related issue arises with geom_abline (with the difference that the warning is correct in that case -- mapping is ignored):

library(ggplot2)

ggplot(data.frame(x = 0), aes(x = x)) +
  geom_abline(intercept=0, slope = 1, aes(color="true value"))

ggplot2 * 3.3.0.9000 2020-01-22 [1] Github (tidyverse/ggplot2@214f314):

geom_abline(): Ignoring `mapping` because `slope` and/or `intercept` were provided.

ggplot2 * 3.2.1 2019-08-10 [1] CRAN (R 3.6.1):

Warning message:
In geom_abline(intercept = 0, slope = 1, aes(color = "true value")) :
  Using `intercept` and/or `slope` with `mapping` may not have the desired result as mapping is overwritten if either of these is specified

image

But here a simple workaround is possible by specifying intercept and slope through mapping:

ggplot(data.frame(x = 0), aes(x = x)) +
  geom_abline(aes(intercept=0, slope = 1, color="true value"))

image


Not sure if this belongs here, but I'm leaving this here since that was the only relevant page that showed up googling above error message and using stat_function as described here would pose another workaround (with warning) (and above workaround would help avoid the warning for stat_function in case of a linear function).


Some thoughts on this issue itself:
Wouldn't thinks be much easier/consistent if fun of stat_function was an aesthetic & inherit.aes would be FALSE (like in geom_abline)?
Shouldn't the warning disappear at least for stat_function(..., inherit.aes = FALSE)?

@tvatter
Copy link

tvatter commented Apr 14, 2020

What is the status of this one? I second the latest suggestion: why is the warning not disappearing when inherit.aes = FALSE ?

@clauswilke
Copy link
Member Author

clauswilke commented May 1, 2020

Just came across this on Stackoverflow:
https://stackoverflow.com/q/61505199/4975218
Maybe we can still fix this for 3.3.1.

@paleolimbot If the concern is that some mappings create multiple copies of the function, then I'm not sure the current warning addresses that issue, because most people won't make the connection between the warning and that problem. Instead, it seems to me that what we need is a geom_function() that works like geom_line() but ensures it draws only once per panel and issues a warning otherwise. If you agree I can take a stab at implementing this.

@clauswilke clauswilke added this to the ggplot2 3.3.1 milestone May 1, 2020
@paleolimbot
Copy link
Member

paleolimbot commented May 1, 2020

@clauswilke absolutely the right way to go! Let me know if there's something I can help with to get the patch release out the door.

clauswilke added a commit to wilkelab/ggplot2_archive that referenced this issue May 1, 2020
@clauswilke
Copy link
Member Author

clauswilke commented May 1, 2020

@paleolimbot Please take a look here: #3982 and let me know what you think.

clauswilke added a commit that referenced this issue May 2, 2020
* Implement geom_function(). Closes #3611.

* update unit test

* rename test file, check for empty data

* improve documentation

* consolidate news items
@clauswilke
Copy link
Member Author

clauswilke commented May 2, 2020

@jan-glx The point you're making regarding geom_abline() is a separate problem. Feel free to open an issue so we won't forget about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants