From cf5877182dbe647be5e4e8eccf3e70fa4a769e48 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 11:36:54 +1100 Subject: [PATCH 1/9] Use transparent scatter lines for errorbars. Fixes #513. --- R/layers2traces.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/layers2traces.R b/R/layers2traces.R index 0ea3b0d53f..f3481c2a01 100644 --- a/R/layers2traces.R +++ b/R/layers2traces.R @@ -655,8 +655,8 @@ make_error <- function(data, params, xy = "x") { text = data$hovertext, type = "scatter", mode = "lines", - opacity = 0, - line = list(color = color) + opacity = 1, + line = list(color = "transparent") ) e[[paste0("error_", xy)]] <- list( array = data[[paste0(xy, "max")]] - data[[xy]], From a9aa06e7b6533136eafb914cfc9f3c31a04eddc6 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 12:00:58 +1100 Subject: [PATCH 2/9] opacity should respect alpha --- R/layers2traces.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/layers2traces.R b/R/layers2traces.R index f3481c2a01..ffd85d9917 100644 --- a/R/layers2traces.R +++ b/R/layers2traces.R @@ -655,7 +655,7 @@ make_error <- function(data, params, xy = "x") { text = data$hovertext, type = "scatter", mode = "lines", - opacity = 1, + opacity = aes2plotly(data, params, "alpha"), line = list(color = "transparent") ) e[[paste0("error_", xy)]] <- list( From 81f46c1c1269d01783e10c5c503dc03146b9f5df Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 12:09:06 +1100 Subject: [PATCH 3/9] add test --- tests/testthat/test-mean-error-bars.R | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/testthat/test-mean-error-bars.R b/tests/testthat/test-mean-error-bars.R index 8fe7f423bb..bf63f57765 100644 --- a/tests/testthat/test-mean-error-bars.R +++ b/tests/testthat/test-mean-error-bars.R @@ -36,3 +36,21 @@ test_that("different colors for error bars, points, and lines", { geom_point(color = "blue", size = 14) L <- save_outputs(one.line.gg, "error-simple-line-point-crazy") }) + +# example from https://github.com/ropensci/plotly/issues/513 + +d <- data.frame( + x = 1:5, + y = 1:5, + label = letters[1:5], + group = factor(c('one', 'one', 'two', 'two', 'one')) +) +fig1 <- ggplot(d, aes(x = x, y = y, text = label, colour = group)) + + geom_rect(fill = 'lightgrey', colour = 'lightgrey', + xmin = 3, xmax = 4, ymin = -4, ymax = 7) + + geom_point() + geom_errorbarh(aes(xmin = x - 1, xmax = x + 2), alpha = 0.5) + + theme_bw() + +test_that("error bars respect trace ordering", { + L <- save_outputs(fig1, "error-rect-alpha") +}) From 0051d1c0c33aa82338ed7f64458897890025f166 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 14:42:43 +1100 Subject: [PATCH 4/9] tests can now handle differencing with different plotlyjs versions --- tests/testthat.R | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/testthat.R b/tests/testthat.R index 8b505e0e83..760197136e 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -12,7 +12,8 @@ if (report_diffs || build_table) { message("Spinning up an independent R session with plotly's master branch installed") Rserve::Rserve(args = "--vanilla --RS-enable-remote") conn <- RSconnect() - # master version should _always_ depend on the CRAN version of ggplot2 + # we don't make assumptions about ggplot2 versioning, + # but it is _strongly_ recommended to use the CRAN version (of ggplot2) RSeval(conn, "devtools::install_github('ropensci/plotly')") RSeval(conn, "library(plotly)") if (report_diffs) { @@ -23,6 +24,18 @@ if (report_diffs || build_table) { master_hash <- substr(master_hash, 1, 7) # plotly-test-table repo hosts the diff pages & keeps track of previous versions table_dir <- normalizePath("../../plotly-test-table", mustWork = T) + # Make sure we have appropriate versions of plotlyjs + # (see plotly-test-table/template/template/index.html) + file.copy( + file.path("..", "inst", "htmlwidgets", "lib", "plotlyjs", "plotly-latest.min.js"), + file.path(table_dir, "template", "New.min.js"), + overwrite = TRUE + ) + download.file( + "https://raw.githubusercontent.com/ropensci/plotly/master/inst/htmlwidgets/lib/plotlyjs/plotly-latest.min.js", + file.path(table_dir, "template", "Old.min.js") + ) + # directory for placing test differences this_dir <- file.path(table_dir, this_hash) if (dir.exists(this_dir)) { message("Tests were already run on this commit. Nuking the old results...") @@ -102,7 +115,7 @@ save_outputs <- function(gg, name) { dir.create(test_dir, recursive = T) # copy over diffing template file.copy( - file.path(table_dir, "template", "template", "index.html"), + file.path(table_dir, "template", "index.html"), test_dir, recursive = T ) From 4d863a6b572e84977c4f220385a401e2f94c0a55 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 14:47:07 +1100 Subject: [PATCH 5/9] try downloading with curl --- tests/testthat.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testthat.R b/tests/testthat.R index 760197136e..eed79934e6 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -33,7 +33,8 @@ if (report_diffs || build_table) { ) download.file( "https://raw.githubusercontent.com/ropensci/plotly/master/inst/htmlwidgets/lib/plotlyjs/plotly-latest.min.js", - file.path(table_dir, "template", "Old.min.js") + file.path(table_dir, "template", "Old.min.js"), + method = "curl" ) # directory for placing test differences this_dir <- file.path(table_dir, this_hash) From e4fc30b8febc31e5c2c57e3ef323737ba54ea0a3 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 15:02:13 +1100 Subject: [PATCH 6/9] template requires copying multiple files now --- tests/testthat.R | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/testthat.R b/tests/testthat.R index eed79934e6..48e928f067 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -116,9 +116,7 @@ save_outputs <- function(gg, name) { dir.create(test_dir, recursive = T) # copy over diffing template file.copy( - file.path(table_dir, "template", "index.html"), - test_dir, - recursive = T + dir(file.path(table_dir, "template"), full.names = TRUE), test_dir ) # overwrite the default JSON writeLines( From 4fa2e5f9386dded048aacea39e08bc0a784dccd6 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 15:56:47 +1100 Subject: [PATCH 7/9] fun with file paths --- tests/testthat.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat.R b/tests/testthat.R index 48e928f067..6cd26602ae 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -116,7 +116,7 @@ save_outputs <- function(gg, name) { dir.create(test_dir, recursive = T) # copy over diffing template file.copy( - dir(file.path(table_dir, "template"), full.names = TRUE), test_dir + dir(file.path(table_dir, "template", "template"), full.names = TRUE), test_dir ) # overwrite the default JSON writeLines( From f41e69a77cdf190afef02869fb65a82439d92e23 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 16:00:07 +1100 Subject: [PATCH 8/9] fix random false positives while we're at it --- tests/testthat.R | 3 +++ tests/testthat/test-ggplot-jitter.R | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testthat.R b/tests/testthat.R index 6cd26602ae..2260d9352f 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -12,6 +12,9 @@ if (report_diffs || build_table) { message("Spinning up an independent R session with plotly's master branch installed") Rserve::Rserve(args = "--vanilla --RS-enable-remote") conn <- RSconnect() + # ensure the seed is the same for randomized tests + set.seed(1) + RSeval(conn, "set.seed(1)") # we don't make assumptions about ggplot2 versioning, # but it is _strongly_ recommended to use the CRAN version (of ggplot2) RSeval(conn, "devtools::install_github('ropensci/plotly')") diff --git a/tests/testthat/test-ggplot-jitter.R b/tests/testthat/test-ggplot-jitter.R index b995795be4..e59e294332 100644 --- a/tests/testthat/test-ggplot-jitter.R +++ b/tests/testthat/test-ggplot-jitter.R @@ -15,7 +15,6 @@ expect_traces <- function(gg, n_traces, name) { list(traces = has_data, layout = L$layout) } -set.seed(1001) p <- ggplot(mpg, aes(cyl, hwy)) + geom_jitter() test_that("geom_jitter is working", { From 2681972b343a1a05f2897b4e12acf1964ce09871 Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Fri, 1 Apr 2016 17:04:41 +1100 Subject: [PATCH 9/9] bump version; update news --- DESCRIPTION | 2 +- NEWS | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 2044001d3b..09bf952191 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: plotly Title: Create Interactive Web Graphics via 'plotly.js' -Version: 3.4.9 +Version: 3.4.10 Authors@R: c(person("Carson", "Sievert", role = c("aut", "cre"), email = "cpsievert1@gmail.com"), person("Chris", "Parmer", role = c("aut", "cph"), diff --git a/NEWS b/NEWS index b7a5c25063..93293f2814 100644 --- a/NEWS +++ b/NEWS @@ -1,10 +1,15 @@ +3.4.10 -- 1 Apr 2016 + +BUGFIX: + +Fix a geom_errorbar bug introduced in 3.4.9. See #513. + 3.4.9 -- 25 Mar 2016 BUGFIX: Upgrade to plotlyjs 1.7.0. Fixes #513 - 3.4.8 -- 23 Mar 2016 BUGFIX: