From e6f19013e0787f7500d9a79905f6fd6e76aebc2f Mon Sep 17 00:00:00 2001 From: Abdessabour Moutik Date: Fri, 22 Oct 2021 14:07:52 -0700 Subject: [PATCH 1/4] Fixed the way `GeomStep` was handeled. Closes #2011 --- R/layers2traces.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/R/layers2traces.R b/R/layers2traces.R index 8ddd90c156..d36a318645 100644 --- a/R/layers2traces.R +++ b/R/layers2traces.R @@ -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(ggplot2:::stairstep(data, direction = params$direction), "GeomPath") } #' @export @@ -623,7 +623,8 @@ to_basic.GeomQuantile <- function(data, prestats_data, layout, params, p, ...){ } #' @export -to_basic.default <- function(data, prestats_data, layout, params, p, ...) { +to_basic.default<- function(data, prestats_data, layout, params, p, ...) { + data } From 943304b714bebafc00fdc5998c334a577b41c13e Mon Sep 17 00:00:00 2001 From: Abdessabour Moutik Date: Fri, 22 Oct 2021 14:27:48 -0700 Subject: [PATCH 2/4] Fixed some issues with the tests. And added new ones. --- R/layers2traces.R | 2 +- tests/testthat/_snaps/ggplot-ecdf/step-ecdf.svg | 1 + tests/testthat/_snaps/ggplot-step/step-gg-hv.svg | 2 +- tests/testthat/_snaps/ggplot-step/step-gg-hvh.svg | 2 +- tests/testthat/_snaps/ggplot-step/step-gg-vh.svg | 2 +- tests/testthat/_snaps/ggplot-step/step-gg-vhv.svg | 2 +- tests/testthat/test-ggplot-ecdf.R | 11 +++++++++++ 7 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 tests/testthat/_snaps/ggplot-ecdf/step-ecdf.svg create mode 100644 tests/testthat/test-ggplot-ecdf.R diff --git a/R/layers2traces.R b/R/layers2traces.R index d36a318645..71542c2544 100644 --- a/R/layers2traces.R +++ b/R/layers2traces.R @@ -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(ggplot2:::stairstep(data, direction = params$direction), "GeomPath") + prefix_class(if((params$direction %||% "vh") %in% c("vh", "hv", "mid")) ggplot2:::stairstep(data, direction = params$direction) else data, c("GeomPath", "GeomStep")) } #' @export diff --git a/tests/testthat/_snaps/ggplot-ecdf/step-ecdf.svg b/tests/testthat/_snaps/ggplot-ecdf/step-ecdf.svg new file mode 100644 index 0000000000..a4a5a14e37 --- /dev/null +++ b/tests/testthat/_snaps/ggplot-ecdf/step-ecdf.svg @@ -0,0 +1 @@ +-200200.000.250.500.751.00xy diff --git a/tests/testthat/_snaps/ggplot-step/step-gg-hv.svg b/tests/testthat/_snaps/ggplot-step/step-gg-hv.svg index f071efdc59..7579bd35c2 100644 --- a/tests/testthat/_snaps/ggplot-step/step-gg-hv.svg +++ b/tests/testthat/_snaps/ggplot-step/step-gg-hv.svg @@ -1 +1 @@ -4008001200160050100150200factor(Tree)12agecircumference +4008001200160050100150200factor(Tree)12agecircumference diff --git a/tests/testthat/_snaps/ggplot-step/step-gg-hvh.svg b/tests/testthat/_snaps/ggplot-step/step-gg-hvh.svg index cce490a084..df9ef867f5 100644 --- a/tests/testthat/_snaps/ggplot-step/step-gg-hvh.svg +++ b/tests/testthat/_snaps/ggplot-step/step-gg-hvh.svg @@ -1 +1 @@ -4008001200160050100150200factor(Tree)12agecircumference +4008001200160050100150200factor(Tree)12agecircumference diff --git a/tests/testthat/_snaps/ggplot-step/step-gg-vh.svg b/tests/testthat/_snaps/ggplot-step/step-gg-vh.svg index a4578fbc24..3c885daccf 100644 --- a/tests/testthat/_snaps/ggplot-step/step-gg-vh.svg +++ b/tests/testthat/_snaps/ggplot-step/step-gg-vh.svg @@ -1 +1 @@ -4008001200160050100150200factor(Tree)12agecircumference +4008001200160050100150200factor(Tree)12agecircumference diff --git a/tests/testthat/_snaps/ggplot-step/step-gg-vhv.svg b/tests/testthat/_snaps/ggplot-step/step-gg-vhv.svg index 9f7b982c0e..ddcc0f4e6b 100644 --- a/tests/testthat/_snaps/ggplot-step/step-gg-vhv.svg +++ b/tests/testthat/_snaps/ggplot-step/step-gg-vhv.svg @@ -1 +1 @@ -4008001200160050100150200factor(Tree)12agecircumference +4008001200160050100150200factor(Tree)12agecircumference diff --git a/tests/testthat/test-ggplot-ecdf.R b/tests/testthat/test-ggplot-ecdf.R new file mode 100644 index 0000000000..6cad39ddcf --- /dev/null +++ b/tests/testthat/test-ggplot-ecdf.R @@ -0,0 +1,11 @@ +test_that("`stat_ecdf` renders correctly", { + df <- data.frame( + x = c(rnorm(100, 0, 3), rnorm(100, 0, 10)), + g = gl(2, 100) + ) + + p <- ggplot(df, aes(x)) + + stat_ecdf(geom = "step") + + expect_doppelganger(ggplotly(p), "step-ecdf") +}) \ No newline at end of file From 660be52701c8d691a92ac15ccfaad38542494fb4 Mon Sep 17 00:00:00 2001 From: Abdessabour Moutik Date: Fri, 22 Oct 2021 14:30:42 -0700 Subject: [PATCH 3/4] Added bug fix to NEWS.md --- NEWS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.md b/NEWS.md index 56775ea13a..69fd00e1cc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,11 @@ * `ggplotly()` does not issue warnings with `options(warnPartialMatchArgs = TRUE)` any longer. (#2046, @bersbersbers) +## BUG fixes + +* `stat_ecdf()` now gives the correct output. (#2052) + + # 4.10.0 ## Breaking changes in JavaScript API From c36bd61bc1d8a5cc5d5d551600c3977e07359ef6 Mon Sep 17 00:00:00 2001 From: Abdessabour Moutik Date: Tue, 2 Nov 2021 05:06:56 -0700 Subject: [PATCH 4/4] Conformed with carson's review --- R/layers2traces.R | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/R/layers2traces.R b/R/layers2traces.R index 71542c2544..bf9881dcc4 100644 --- a/R/layers2traces.R +++ b/R/layers2traces.R @@ -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(if((params$direction %||% "vh") %in% c("vh", "hv", "mid")) ggplot2:::stairstep(data, direction = params$direction) else data, c("GeomPath", "GeomStep")) + prefix_class(if((params$direction %||% "vh") %in% c("vh", "hv", "mid")) ggfun("stairstep")(data, direction = params$direction) else data, c("GeomPath", "GeomStep")) } #' @export @@ -623,8 +623,7 @@ to_basic.GeomQuantile <- function(data, prestats_data, layout, params, p, ...){ } #' @export -to_basic.default<- function(data, prestats_data, layout, params, p, ...) { - +to_basic.default <- function(data, prestats_data, layout, params, p, ...) { data }