Update stat_ecdf to work either on the x or the y aesthetic.

[jgjl]

• Extend stat_ecdf to detect the input and output aesthetics
• Add simple test for the direction selection mechanism, modeled after stat_density

Not sure about the completeness of the documentation.

[yutannihilation]

I agree stat_ecdf() should have flipping functionality, but changing the computed variables is considered a breaking change, and I'm not sure if this worth it...

[jgjl]

Thank you for taking a look!
I aimed for modifying the implementation as has been done for other stats before and suggested by @clauswilke https://github.com/jgjl?tab=overview&from=2020-02-01&to=2020-02-29.
Do you think there is a better, less obtrusive way to get there? My experience with the implementational details of ggplot is limited.

[yutannihilation]

Thanks for the context. Most of the Stats provides variable names that are not related to the orientation, so replacing y instead of ecdf sounds consistent with the manner. That said, again, I'm afraid it's not worth a breaking change.

After some tweaking on my local, I think you can keep using y as below. You can consider only about the unflipped case between two flip_data(), and default_aes() will be flipped on Geom's side, iiuc. What about this?

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

Also, I suggest you open an issue first describing the feature you'd like to add, and then you can make a new PR implementing that feature.

So, you are supposed to open an issue first instead of a direct pull request. (Just for future reference, no worries this time!)

[jgjl]

Your proposal sounds good to me. My commit should update the branch to your proposed approach.

Sorry for not opening the issue!

[yutannihilation]

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).

[jgjl]

I added the line, quite brief, but it should do the trick I hope.
Thank you for the feedback and guidance!

[yutannihilation]

/document

[yutannihilation]

Looks good. Thanks!