-
Notifications
You must be signed in to change notification settings - Fork 633
Add stat_ecdf support #2052
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
Add stat_ecdf support #2052
Conversation
R/layers2traces.R
Outdated
to_basic.default<- function(data, prestats_data, layout, params, p, ...) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to_basic.default<- function(data, prestats_data, layout, params, p, ...) { | |
to_basic.default <- function(data, prestats_data, layout, params, p, ...) { |
R/layers2traces.R
Outdated
@@ -252,7 +252,7 @@ to_basic.GeomLine <- function(data, prestats_data, layout, params, p, ...) { | |||
|
|||
#' @export | |||
to_basic.GeomStep <- function(data, prestats_data, layout, params, p, ...) { | |||
prefix_class(data, "GeomPath") | |||
prefix_class(if((params$direction %||% "vh") %in% c("vh", "hv", "mid")) ggplot2:::stairstep(data, direction = params$direction) else data, c("GeomPath", "GeomStep")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using :::
isn't allowed by R CMD check
. Use ggfun()
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this breaks default geom_step()
usage. See, for example, the change in tests/testthat/_snaps/ggplot-step/step-gg-hv.svg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I fixed the test issues. basically just re rendering the svg solved it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the tests are still broken. I don't think this is the right fix.
This isn't the right approach, I'll take this on in #2065 |
NEWS.md