Allow rlang-style lambda expressions in stat_summary functions #3569
Conversation
dkahle
changed the title
R/stat-summary-bin.R
Outdated
| fun.data <- match.fun(fun.data) | ||
| if (is.formula(fun.data)) { | ||
| fun.data <- rlang::as_function(fun.data) | ||
| } else { | ||
| fun.data <- match.fun(fun.data) | ||
| } |
There was a problem hiding this comment.
I think you can just use as_function() for everything as it behaves correctly in all cases... We also import rlang so no need to prefix it
Agreed. |
|
@dkahle do you want to finish this off? |
|
@thomasp85 Yep! Is that what you're looking for? |
R/stat-summary-2d.r
Outdated
| @@ -100,7 +100,7 @@ StatSummary2d <- ggproto("StatSummary2d", Stat, | |||
| xbin <- cut(data$x, xbreaks, include.lowest = TRUE, labels = FALSE) | |||
| ybin <- cut(data$y, ybreaks, include.lowest = TRUE, labels = FALSE) | |||
|
|
|||
| if (is.formula(fun)) fun <- rlang::as_function(fun) | |||
| if (is.formula(fun)) fun <- as_function(fun) | |||
There was a problem hiding this comment.
I think you can just remove the is.formula check and call as_function() unconditionally... same with the other two places
|
Apart from the small comment you just need to fix the conflict in the news file, and it is good to go... thanks |
|
Hm. I pulled the news from upstream/master, updated it, and it still lists it as a conflict. What's the best way to resolve that? (Other than not wait two months before addressing your comments, that is.:) |
|
You should merge the master branch into this branch (make sure master is current) And don’t worry about time-to-respond. We all have shifting focus🙂 |
Merge branch 'master' of https://github.com/tidyverse/ggplot2 into rlang-lambda-stat-summary # Conflicts: # NEWS.md
|
You should fix the merge conflict after merging in master. You still have the conflict in the NEWS file (the weird Are you familiar with fixing merge conflicts or should I walk you through it? |
|
Just seeing your second comment. I am familiar with merge conflicts; I thought I removed all merge conflict notes - did I miss something? |
NEWS.md
Outdated
| @@ -1,5 +1,19 @@ | |||
| # ggplot2 (development version) | |||
|
|
|||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
Why yes, yes there is. 😂 I feel like this is the second time that my commit message should have a facepalm emoji.
I actually searched the project for <<<<, but I realize now I was only searching through the R script files... Stay tuned. Thanks!
Merge branch 'master' of https://github.com/tidyverse/ggplot2 into rlang-lambda-stat-summary # Conflicts: # NEWS.md
|
Thank you! |
No description provided.