From 2e9546abc80b416f54021c352f8edf46220666b3 Mon Sep 17 00:00:00 2001 From: Baobao Zhang <13bzhang@gmail.com> Date: Wed, 10 Jun 2015 05:12:39 -0400 Subject: [PATCH 1/8] added legends for boxplots and densities --- R/ggplotly.R | 8 ++++-- tests/testthat/test-ggplot-boxplot.R | 42 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index 13c80ddf30..347d777cbe 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -57,7 +57,9 @@ markLegends <- ## group, even when they have the exact same visual ## characteristics and could be drawn using just 1 trace! polygon=c("colour", "fill", "linetype", "size"), - bar=c("colour", "fill"), + bar = c("colour", "fill"), + #boxplot = c("colour", "fill"), + density = c("colour", "fill", "linetype"), errorbar=c("colour", "linetype"), errorbarh=c("colour", "linetype"), area=c("colour", "fill"), @@ -245,7 +247,9 @@ gg2list <- function(p) { # This extracts essential info for this geom/layer. traces <- layer2traces(L, df, misc) - possible.legends <- markLegends[[L$geom$objname]] + possible.legends <- ifelse(L$geom$objname == "boxplot", + markSplit[[L$geom$objname]], + markLegends[[L$geom$objname]]) actual.legends <- possible.legends[possible.legends %in% names(L$mapping)] layer.legends[[paste(i)]] <- actual.legends diff --git a/tests/testthat/test-ggplot-boxplot.R b/tests/testthat/test-ggplot-boxplot.R index ef9ac283f7..0aa11c7ac3 100644 --- a/tests/testthat/test-ggplot-boxplot.R +++ b/tests/testthat/test-ggplot-boxplot.R @@ -1,5 +1,19 @@ context("Boxplot") +expect_traces <- function(gg, n.traces, name) { + stopifnot(is.ggplot(gg)) + stopifnot(is.numeric(n.traces)) + save_outputs(gg, paste0("boxplot-", name)) + L <- gg2list(gg) + all.traces <- L$data + no.data <- sapply(all.traces, function(tr) { + is.null(tr[["x"]]) && is.null(tr[["y"]]) + }) + has.data <- all.traces[!no.data] + expect_equal(length(has.data), n.traces) + list(traces=has.data, layout=L$layout) +} + test_that("geom_boxplot gives a boxplot", { gg <- ggplot(mtcars, aes(factor(cyl), mpg)) + geom_boxplot() @@ -48,3 +62,31 @@ test_that("you can make a boxplot for a distribution of datetimes", { save_outputs(bp, "boxplot-datetime") }) + +# check legend shows up when each box-and-whiskers has a fill +# make ggplot2 +m <- ggplot(mtcars, aes(factor(cyl), mpg)) +p <- m + geom_boxplot(aes(fill = factor(cyl))) +# tests +test_that("legends for boxplot", { + info <- expect_traces(p, 3, "legends_for_fill") + tr <- info$traces + la <- info$layout + expect_identical(tr[[1]]$type, "box") + # check legend exists + expect_identical(la$showlegend, TRUE) + # check legend for each fill exists + for (i in 1:3) { + expect_identical(tr[[i]]$showlegend, TRUE) + } + # check the fill colors are correct + g <- ggplot_build(p) + fill.colors <- unique(g$data[[1]]["fill"])[,1] + for (i in 1:3) { + plotly.color <- as.integer(strsplit(gsub("[\\(\\)]|rgb", "", + tr[[i]]$fillcolor), split = ",")[[1]]) + names(plotly.color) <- c("red", "green", "blue") + expect_equal(plotly.color, col2rgb(fill.colors[i])[,1], + tolerance = 1) + } +}) \ No newline at end of file From 1d2c18ac61b7880e79f231d87d71b0945ae80858 Mon Sep 17 00:00:00 2001 From: Baobao Zhang <13bzhang@gmail.com> Date: Wed, 10 Jun 2015 05:14:30 -0400 Subject: [PATCH 2/8] added extra spaces to the end of boxplot test --- tests/testthat/test-ggplot-boxplot.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-ggplot-boxplot.R b/tests/testthat/test-ggplot-boxplot.R index 0aa11c7ac3..6ecf17b7ad 100644 --- a/tests/testthat/test-ggplot-boxplot.R +++ b/tests/testthat/test-ggplot-boxplot.R @@ -89,4 +89,6 @@ test_that("legends for boxplot", { expect_equal(plotly.color, col2rgb(fill.colors[i])[,1], tolerance = 1) } -}) \ No newline at end of file +}) + + From 6dfa9dad5218dc9c980e9d9d12de6832499bf7a5 Mon Sep 17 00:00:00 2001 From: Baobao Zhang <13bzhang@gmail.com> Date: Wed, 10 Jun 2015 05:27:38 -0400 Subject: [PATCH 3/8] changed if statement --- R/ggplotly.R | 8 ++++---- tests/testthat/test-ggplot-trace-order.R | 0 2 files changed, 4 insertions(+), 4 deletions(-) create mode 100644 tests/testthat/test-ggplot-trace-order.R diff --git a/R/ggplotly.R b/R/ggplotly.R index 347d777cbe..3600b61bf9 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -58,7 +58,6 @@ markLegends <- ## characteristics and could be drawn using just 1 trace! polygon=c("colour", "fill", "linetype", "size"), bar = c("colour", "fill"), - #boxplot = c("colour", "fill"), density = c("colour", "fill", "linetype"), errorbar=c("colour", "linetype"), errorbarh=c("colour", "linetype"), @@ -247,9 +246,10 @@ gg2list <- function(p) { # This extracts essential info for this geom/layer. traces <- layer2traces(L, df, misc) - possible.legends <- ifelse(L$geom$objname == "boxplot", - markSplit[[L$geom$objname]], - markLegends[[L$geom$objname]]) + possible.legends <- markLegends[[L$geom$objname]] + if (L$geom$objname == "boxplot"){ + possible.legends <- markSplit[[L$geom$objname]] + } actual.legends <- possible.legends[possible.legends %in% names(L$mapping)] layer.legends[[paste(i)]] <- actual.legends diff --git a/tests/testthat/test-ggplot-trace-order.R b/tests/testthat/test-ggplot-trace-order.R new file mode 100644 index 0000000000..e69de29bb2 From 7b6f87feff32121a2f6503a386d1a0a5c6ea8561 Mon Sep 17 00:00:00 2001 From: Baobao Zhang <13bzhang@gmail.com> Date: Thu, 11 Jun 2015 23:40:33 -0400 Subject: [PATCH 4/8] add legend tests to density test --- tests/testthat/test-ggplot-density.R | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/testthat/test-ggplot-density.R b/tests/testthat/test-ggplot-density.R index 93a1752ea8..55766fb01b 100644 --- a/tests/testthat/test-ggplot-density.R +++ b/tests/testthat/test-ggplot-density.R @@ -29,10 +29,28 @@ test_that("geom_density() respects fill aesthetic", { gg <- base + geom_density(aes(fill=factor(vs)), alpha = 0.3) info <- expect_traces(gg, 2, "fill") trs <- info$traces + la <- info$layout type <- unique(sapply(trs, "[[", "type")) fill <- unique(sapply(trs, "[[", "fill")) expect_identical(type, "scatter") expect_identical(fill, "tozeroy") + # check legend exists + expect_identical(la$showlegend, TRUE) + # check legend for each fill exists + for (i in 1:2) { + expect_identical(trs[[i]]$showlegend, TRUE) + } + # check the fill colors are correct + g <- ggplot_build(gg) + fill.colors <- unique(g$data[[1]]["fill"])[,1] + for (i in 1:2) { + plotly.color <- as.integer( + strsplit(gsub("[\\(\\)]|rgba", "", + trs[[i]]$fillcolor), split = ",")[[1]])[1:3] + names(plotly.color) <- c("red", "green", "blue") + expect_equal(plotly.color, col2rgb(fill.colors[i])[,1], + tolerance = 1) + } }) test_that("geom_density() respects colour aesthetic", { From dbb35bb8970ccf1ccc1a2b31e4f4cff9d212d720 Mon Sep 17 00:00:00 2001 From: cpsievert Date: Thu, 10 Dec 2015 13:27:07 -0800 Subject: [PATCH 5/8] tests that test our internal logic/assumptions are not tests --- tests/testthat/test-ggplot-density.R | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/testthat/test-ggplot-density.R b/tests/testthat/test-ggplot-density.R index 8e5a84f79e..d6396424c6 100644 --- a/tests/testthat/test-ggplot-density.R +++ b/tests/testthat/test-ggplot-density.R @@ -33,22 +33,9 @@ test_that("geom_density() respects fill aesthetic", { expect_identical(type, "scatter") expect_identical(fill, "tozeroy") # check legend exists - expect_identical(la$showlegend, TRUE) + expect_true(info$layout$showlegend, TRUE) # check legend for each fill exists - for (i in 1:2) { - expect_identical(trs[[i]]$showlegend, TRUE) - } - # check the fill colors are correct - g <- ggplot_build(gg) - fill.colors <- unique(g$data[[1]]["fill"])[,1] - for (i in 1:2) { - plotly.color <- as.integer( - strsplit(gsub("[\\(\\)]|rgba", "", - trs[[i]]$fillcolor), split = ",")[[1]])[1:3] - names(plotly.color) <- c("red", "green", "blue") - expect_equal(plotly.color, col2rgb(fill.colors[i])[,1], - tolerance = 1) - } + expect_true(all(sapply(trs, "[[", "showlegend"))) }) test_that("geom_density() respects colour aesthetic", { From 5b077d7642804a00979da343812b0aa6a5dfa154 Mon Sep 17 00:00:00 2001 From: cpsievert Date: Fri, 11 Dec 2015 18:02:22 -0600 Subject: [PATCH 6/8] we don't want a legend for x-values --- R/ggplotly.R | 11 +++++------ R/trace_generation.R | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index 6ef47639d2..3f5c1b89f0 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -105,8 +105,9 @@ markLegends <- ## group, even when they have the exact same visual ## characteristics and could be drawn using just 1 trace! polygon=c("colour", "fill", "linetype", "size"), - bar = c("colour", "fill"), - density = c("colour", "fill", "linetype"), + bar=c("colour", "fill"), + density=c("colour", "fill", "linetype"), + boxplot=c("colour", "fill", "size"), errorbar=c("colour", "linetype"), errorbarh=c("colour", "linetype"), area=c("colour", "fill"), @@ -115,7 +116,8 @@ markLegends <- markUnique <- as.character(unique(unlist(markLegends))) -markSplit <- c(markLegends,list(boxplot=c("x"))) +markSplit <- markLegends +markSplit$boxplot <- c(markSplit$boxplot, "x") guide_names <- function(p, aes = c("shape", "fill", "alpha", "area", "color", "colour", "size", "linetype")) { @@ -306,9 +308,6 @@ gg2list <- function(p) { traces <- layer2traces(L, df, misc) possible.legends <- markLegends[[L$geom$objname]] - if (L$geom$objname == "boxplot"){ - possible.legends <- markSplit[[L$geom$objname]] - } actual.legends <- possible.legends[possible.legends %in% names(L$mapping)] layer.legends[[paste(i)]] <- actual.legends diff --git a/R/trace_generation.R b/R/trace_generation.R index fae9fef4a1..740e311db8 100644 --- a/R/trace_generation.R +++ b/R/trace_generation.R @@ -294,7 +294,7 @@ layer2traces <- function(l, d, misc) { s <- no.sort[[tr.i]]$sort no.sort[[tr.i]]$showlegend <- if (is.numeric(s)) { - if (s == Inf){ + if (all(s %in% Inf)){ FALSE } else { TRUE From 3bf2266f7791dcf21ef7d17c189c08235c2a87b3 Mon Sep 17 00:00:00 2001 From: cpsievert Date: Sun, 13 Dec 2015 15:53:19 -0600 Subject: [PATCH 7/8] avoid redundant spliting --- R/ggplotly.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/ggplotly.R b/R/ggplotly.R index 3f5c1b89f0..17017686be 100644 --- a/R/ggplotly.R +++ b/R/ggplotly.R @@ -117,7 +117,7 @@ markLegends <- markUnique <- as.character(unique(unlist(markLegends))) markSplit <- markLegends -markSplit$boxplot <- c(markSplit$boxplot, "x") +markSplit$boxplot <- "x" guide_names <- function(p, aes = c("shape", "fill", "alpha", "area", "color", "colour", "size", "linetype")) { From 559381b5ad8aa5f5cf8fd820606ef1b614777622 Mon Sep 17 00:00:00 2001 From: cpsievert Date: Sun, 13 Dec 2015 16:18:42 -0600 Subject: [PATCH 8/8] bump version; update news --- DESCRIPTION | 2 +- NEWS | 4 ++++ R/trace_generation.R | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index ca5fe573b0..f9b782d37a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: plotly Title: Create Interactive Web Graphics via Plotly's JavaScript Graphing Library -Version: 2.0.13 +Version: 2.0.14 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 313c4139a6..8a4232d3b8 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +2.0.14 -- 13 Dec 2015 + +Fix #212 + 2.0.13 -- 12 Dec 2015 Fix #286 diff --git a/R/trace_generation.R b/R/trace_generation.R index 02b380a19e..5a8838179c 100644 --- a/R/trace_generation.R +++ b/R/trace_generation.R @@ -294,7 +294,7 @@ layer2traces <- function(l, d, misc) { s <- no.sort[[tr.i]]$sort no.sort[[tr.i]]$showlegend <- if (is.numeric(s)) { - if (all(s %in% Inf)){ + if (s == Inf){ FALSE } else { TRUE