Update stat_ecdf to work either on the x or the y aesthetic. - #4005
Conversation
|
I agree |
|
Thank you for taking a look! |
|
Thanks for the context. Most of the Stats provides variable names that are not related to the orientation, so replacing After some tweaking on my local, I think you can keep using diff --git a/R/stat-ecdf.r b/R/stat-ecdf.r
index 5fab8d73..9a8d73e5 100644
--- a/R/stat-ecdf.r
+++ b/R/stat-ecdf.r
@@ -73,7 +73,7 @@ stat_ecdf <- function(mapping = NULL, data = NULL,
StatEcdf <- ggproto("StatEcdf", Stat,
required_aes = c("x|y"),
- default_aes = aes(x = after_stat(ecdf), y = after_stat(ecdf)),
+ default_aes = aes(y = after_stat(y)),
setup_params = function(data, params) {
params$flipped_aes <- has_flipped_aes(data, params, main_is_orthogonal = FALSE, main_is_continuous = TRUE)
@@ -101,7 +101,7 @@ StatEcdf <- ggproto("StatEcdf", Stat,
}
data_ecdf <- ecdf(data$x)(x)
- df_ecdf <- new_data_frame(list(x = x, ecdf = data_ecdf), n = length(x))
+ df_ecdf <- new_data_frame(list(x = x, y = data_ecdf), n = length(x))
df_ecdf$flipped_aes <- flipped_aes
flip_data(df_ecdf, flipped_aes)
}Btw, on your original PR #3832, @clauswilke said
So, you are supposed to open an issue first instead of a direct pull request. (Just for future reference, no worries this time!) |
|
Your proposal sounds good to me. My commit should update the branch to your proposed approach. Sorry for not opening the issue! |
|
Looks good now! Could you please add a bullet to the top of NEWS.md? It should briefly describe the change and end with (@yourname, #issuenumber). |
|
I added the line, quite brief, but it should do the trick I hope. |
|
/document |
Not sure about the completeness of the documentation.