From c9f7ff295f820b6c40f5143a388d6c51552f0156 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Wed, 18 Mar 2015 17:07:47 -0500
Subject: [PATCH 01/12] Reimplement ribbon as a basic polygon. Fix #191. Fix
 #192.

---
 R/trace_generation.R                | 48 +++++++++++++++++-------
 tests/testthat/test-ggplot-ribbon.R | 57 +++++++++++++++++++++--------
 tests/testthat/test-ggplot-smooth.R | 47 ++++++++++++++++++++----
 3 files changed, 115 insertions(+), 37 deletions(-)

diff --git a/R/trace_generation.R b/R/trace_generation.R
index 2181e0e3c4..5ef45aaf11 100644
--- a/R/trace_generation.R
+++ b/R/trace_generation.R
@@ -26,13 +26,15 @@ layer2traces <- function(l, d, misc) {
   # geom_smooth() means geom_line() + geom_ribbon()
   # Note the line is always drawn, but ribbon is not if se = FALSE.
   if (g$geom == "smooth") {
-    # If smoothLine has been compiled already, consider smoothRibbon.
+    # If smoothLine has been compiled already, consider drawing the ribbon
     if (isTRUE(misc$smoothLine)) {
       misc$smoothLine <- FALSE
       if (isTRUE(l$stat_params$se == FALSE)) {
         return(NULL) 
       } else {
         g$geom <- "smoothRibbon"
+        # disregard colour
+        g$data <- g$data[!grepl("^colour[.name]?", names(g$data))]
       }
     } else {
       misc$smoothLine <- TRUE
@@ -248,7 +250,6 @@ layer2traces <- function(l, d, misc) {
       if (length(unique(name.list)) < 2)
         tr$name <- as.character(name.list[[1]])
     }
-    
     dpd <- data.params$data
     if ("PANEL" %in% names(dpd) && nrow(dpd) > 0)
     {
@@ -335,6 +336,11 @@ toBasic <- list(
     g$geom <- "polygon"
     g
   },
+  ribbon=function(g) {
+    g$data <- ribbon_dat(g$data)
+    g$geom <- "polygon"
+    g
+  },
   path=function(g) {
     group2NA(g, "path")
   },
@@ -410,8 +416,10 @@ toBasic <- list(
     group2NA(g, "path")
   },
   smoothRibbon=function(g) {
-    if (is.null(g$params$alpha)) g$params$alpha <- 0.1
-    group2NA(g, "ribbon")
+    if (is.null(g$params$alpha)) g$params$alpha <- 0.2
+    g$data <- ribbon_dat(g$data)
+    g$geom <- "polygon"
+    g
   }
 )
 
@@ -493,6 +501,26 @@ make.errorbar <- function(data, params, xy){
   tr
 }
 
+# function to transform geom_ribbon data into format plotly likes
+# (note this function is also used for geom_smooth)
+ribbon_dat <- function(dat) {
+  n <- nrow(dat)
+  o <- order(dat$x)
+  o2 <- order(dat$x, decreasing = TRUE)
+  used <- c("x", "ymin", "ymax")
+  not_used <- setdiff(names(dat), used)
+  # top-half of ribbon
+  tmp <- dat[o, ]
+  others <- tmp[not_used]
+  dat1 <- cbind(x = tmp$x, y = tmp$ymax, others)
+  dat1[n+1, ] <- cbind(x = tmp$x[n], y = tmp$ymin[n], others[n, ])
+  # bottom-half of ribbon
+  tmp2 <- dat[o2, ]
+  others2 <- tmp2[not_used]
+  dat2 <- cbind(x = tmp2$x, y = tmp2$ymin, others2)
+  rbind(dat1, dat2)
+}
+
 # Convert basic geoms to traces.
 geom2trace <- list(
   path=function(data, params) {
@@ -515,7 +543,8 @@ geom2trace <- list(
          mode="lines",
          line=paramORdefault(params, aes2line, polygon.line.defaults),
          fill="tozerox",
-         fillcolor=toFill(params$fill))
+         fillcolor=toFill(params$fill, ifelse(is.null(params$alpha), 1,
+                                              params$alpha)))
   },
   point=function(data, params){
     L <- list(x=data$x,
@@ -667,15 +696,6 @@ geom2trace <- list(
          fillcolor=toFill(params$fill, ifelse(is.null(params$alpha), 1,
                                               params$alpha)))
   },
-  ribbon=function(data, params) {
-    list(x=c(data$x[1], data$x, rev(data$x)),
-         y=c(data$ymin[1], data$ymax, rev(data$ymin)),
-         type="scatter",
-         line=paramORdefault(params, aes2line, ribbon.line.defaults),
-         fill="tonexty",
-         fillcolor=toFill(params$fill, ifelse(is.null(params$alpha), 1,
-                                              params$alpha)))
-  },
   abline=function(data, params) {
     list(x=c(params$xstart, params$xend),
          y=c(params$intercept + params$xstart * params$slope,
diff --git a/tests/testthat/test-ggplot-ribbon.R b/tests/testthat/test-ggplot-ribbon.R
index aca320f0bd..562cb8d604 100644
--- a/tests/testthat/test-ggplot-ribbon.R
+++ b/tests/testthat/test-ggplot-ribbon.R
@@ -1,26 +1,51 @@
 context("ribbon")
 
-huron <- data.frame(year=1875:1972, level=as.vector(LakeHuron))
+expect_traces <- function(gg, n.traces, name){
+  stopifnot(is.ggplot(gg))
+  stopifnot(is.numeric(n.traces))
+  save_outputs(gg, paste0("ribbon-", name))
+  L <- gg2list(gg)
+  is.trace <- names(L) == ""
+  all.traces <- L[is.trace]
+  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, kwargs=L$kwargs)
+}
 
-rb <- ggplot(huron, aes(x=year)) + geom_ribbon(aes(ymin=level-1, ymax=level+1))
-L <- gg2list(rb)
+huron <- data.frame(year = 1875:1972, level = as.vector(LakeHuron))
+huron$decade <- with(huron, round(year/10) * 10)
+huron$diff <- huron$year - huron$decade
 
-test_that("sanity check for geom_ribbon", {
-  expect_equal(length(L), 2)
-  expect_identical(L[[1]]$type, "scatter")
-  expect_equal(L[[1]]$x, c(huron$year[1], huron$year, rev(huron$year)))
-  expect_equal(L[[1]]$y, c(huron$level[1]-1, huron$level+1, rev(huron$level-1)))
-  expect_identical(L[[1]]$line$color, "transparent")
+p1 <- ggplot(data = huron) + 
+  geom_ribbon(aes(x = year, ymin = level-1, ymax = level+1), 
+              alpha = 0.1)
+
+test_that("geom_ribbon() creates 1 trace & respects alpha transparency", {
+  info <- expect_traces(p1, 1, "alpha")
+  tr <- info$traces[[1]]
+  expect_match(L2[[1]]$fillcolor, "0.1)", fixed=TRUE)
 })
 
-save_outputs(rb, "ribbon")
+p2 <- ggplot(data = huron, aes(group = factor(decade))) + 
+  geom_ribbon(aes(x = diff, ymin = level-0.1, ymax = level+0.1))
 
-rb2 <- ggplot(huron, aes(x=year)) + 
-  geom_ribbon(aes(ymin=level-1, ymax=level+1), alpha = 0.1)
-L2 <- gg2list(rb2)
+test_that("geom_ribbon() group aesthetic", {
+  info <- expect_traces(p2, 1, "group")
+})
 
-test_that("geom_ribbon respects alpha transparency", {
-  expect_match(L2[[1]]$fillcolor, "0.1)", fixed=TRUE)
+p3 <- ggplot(data = huron, aes(colour = factor(decade))) + 
+  geom_ribbon(aes(x = diff, ymin = level-0.1, ymax = level+0.1))
+
+test_that("geom_ribbon() colour aesthetic", {
+  info <- expect_traces(p3, 1, "colour")
 })
 
-save_outputs(rb2, "ribbon-alpha")
+p4 <- ggplot(data = huron, aes(fill = factor(decade))) + 
+  geom_ribbon(aes(x = diff, ymin = level-0.1, ymax = level+0.1))
+
+test_that("geom_ribbon() fill aesthetic", {
+  info <- expect_traces(p4, 1, "fill")
+})
diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index 7e5cbdb168..b10fe6fb1d 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -1,18 +1,51 @@
 context("smooth")
 
+expect_traces <- function(gg, n.traces, name){
+  stopifnot(is.ggplot(gg))
+  stopifnot(is.numeric(n.traces))
+  save_outputs(gg, paste0("smooth-", name))
+  L <- gg2list(gg)
+  is.trace <- names(L) == ""
+  all.traces <- L[is.trace]
+  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, kwargs=L$kwargs)
+}
+
 p <- ggplot(mtcars, aes(mpg, wt)) + geom_point() + geom_smooth()
 
 test_that("geom_point() + geom_smooth() produces 3 traces", {
-  info <- gg2list(p)
-  expect_true(sum(names(info) == "") == 3)
-  save_outputs(p, "smooth")
+  expect_traces(p, 3, "basic")
 })
 
-p2 <- ggplot(mtcars, aes(mpg, wt)) + geom_point() + geom_smooth(se = FALSE)
+p2 <- ggplot(mtcars, aes(mpg, wt)) + geom_point() + 
+  geom_smooth(se = FALSE)
 
 test_that("geom_point() + geom_smooth(se = FALSE) produces 2 traces", {
-  info2 <- gg2list(p2)
-  expect_true(sum(names(info2) == "") == 2)
-  save_outputs(p2, "smooth-se-false")
+  expect_traces(p2, 2, "se-false")
 })
 
+d <- diamonds[sample(nrows(diamonds, 1000)), ]
+p3 <- qplot(carat, price, group = cut, data = d) + geom_smooth()
+
+test_that("geom_smooth() respects group aesthetic", {
+  # 1 trace for points
+  # 5 traces for lines (1 for each group)
+  # 5 traces for ribbons (1 for each group)
+  expect_traces(p3, 11, "group")
+})
+
+p4 <- qplot(carat, price, colour = cut, data = d) + geom_smooth()
+
+test_that("geom_smooth() respects colour aesthetic", {
+  expect_traces(p4, 11, "colour")
+})
+
+p5 <- qplot(carat, price, fill = cut, data = d) + geom_smooth()
+
+test_that("geom_smooth() respects fill aesthetic", {
+  expect_traces(p5, 11, "fill")
+})

From 4ef1709f0f7403af7622d3c1ec8f1eccaf5e4dc7 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Wed, 18 Mar 2015 17:09:46 -0500
Subject: [PATCH 02/12] NEWS/DESCRIPTION

---
 DESCRIPTION                         | 2 +-
 NEWS                                | 4 ++++
 tests/testthat/test-ggplot-smooth.R | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 18e50e1b92..02d7831a9a 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,7 +1,7 @@
 Package: plotly
 Type: Package
 Title: Interactive, publication-quality graphs online.
-Version: 0.5.26
+Version: 0.5.27
 Authors@R: c(person("Chris", "Parmer", role = c("aut", "cre"),
     email = "chris@plot.ly"),
     person("Scott", "Chamberlain", role = "aut",
diff --git a/NEWS b/NEWS
index d1d6bdca77..cefff8b5e7 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+0.5.27 -- 19 Mar 2015
+
+Reimplement geom_ribbon as a basic polygon. Fix #191. Fix #192.
+
 0.5.26 -- 18 Mar 2015
 
 Implemented geom_rect #178
diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index b10fe6fb1d..c215214e73 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -28,7 +28,7 @@ test_that("geom_point() + geom_smooth(se = FALSE) produces 2 traces", {
   expect_traces(p2, 2, "se-false")
 })
 
-d <- diamonds[sample(nrows(diamonds, 1000)), ]
+d <- diamonds[sample(nrow(diamonds), 1000), ]
 p3 <- qplot(carat, price, group = cut, data = d) + geom_smooth()
 
 test_that("geom_smooth() respects group aesthetic", {

From 9aacdf05d536ee2d93ea4e5d31aac453515df7e1 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Wed, 18 Mar 2015 17:19:38 -0500
Subject: [PATCH 03/12] get basic tests working

---
 tests/testthat/test-ggplot-ribbon.R | 12 +++++++-----
 tests/testthat/test-ggplot-smooth.R |  6 +++---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/tests/testthat/test-ggplot-ribbon.R b/tests/testthat/test-ggplot-ribbon.R
index 562cb8d604..736eaa9fb7 100644
--- a/tests/testthat/test-ggplot-ribbon.R
+++ b/tests/testthat/test-ggplot-ribbon.R
@@ -32,20 +32,22 @@ test_that("geom_ribbon() creates 1 trace & respects alpha transparency", {
 p2 <- ggplot(data = huron, aes(group = factor(decade))) + 
   geom_ribbon(aes(x = diff, ymin = level-0.1, ymax = level+0.1))
 
-test_that("geom_ribbon() group aesthetic", {
+test_that("geom_ribbon() with group aesthetic produces 1 trace", {
   info <- expect_traces(p2, 1, "group")
 })
 
 p3 <- ggplot(data = huron, aes(colour = factor(decade))) + 
   geom_ribbon(aes(x = diff, ymin = level-0.1, ymax = level+0.1))
 
-test_that("geom_ribbon() colour aesthetic", {
-  info <- expect_traces(p3, 1, "colour")
+test_that("geom_ribbon() with colour aesthetic produces multiple traces", {
+  # 10 traces -- one for each decade
+  info <- expect_traces(p3, 10, "colour")
 })
 
 p4 <- ggplot(data = huron, aes(fill = factor(decade))) + 
   geom_ribbon(aes(x = diff, ymin = level-0.1, ymax = level+0.1))
 
-test_that("geom_ribbon() fill aesthetic", {
-  info <- expect_traces(p4, 1, "fill")
+test_that("geom_ribbon() with fill aesthetic produces multiple traces", {
+  # 10 traces -- one for each decade
+  info <- expect_traces(p4, 10, "fill")
 })
diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index c215214e73..72f68d5373 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -35,17 +35,17 @@ test_that("geom_smooth() respects group aesthetic", {
   # 1 trace for points
   # 5 traces for lines (1 for each group)
   # 5 traces for ribbons (1 for each group)
-  expect_traces(p3, 11, "group")
+  info <- expect_traces(p3, 11, "group")
 })
 
 p4 <- qplot(carat, price, colour = cut, data = d) + geom_smooth()
 
 test_that("geom_smooth() respects colour aesthetic", {
-  expect_traces(p4, 11, "colour")
+  info <- expect_traces(p4, 11, "colour")
 })
 
 p5 <- qplot(carat, price, fill = cut, data = d) + geom_smooth()
 
 test_that("geom_smooth() respects fill aesthetic", {
-  expect_traces(p5, 11, "fill")
+  info <- expect_traces(p5, 11, "fill")
 })

From 351024709ba667375978552ebfa2f0370fa1bd30 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Wed, 18 Mar 2015 17:52:38 -0500
Subject: [PATCH 04/12] actually get basic tests working

---
 tests/testthat/test-ggplot-smooth.R | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index 72f68d5373..04e96e8d5a 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -32,10 +32,7 @@ d <- diamonds[sample(nrow(diamonds), 1000), ]
 p3 <- qplot(carat, price, group = cut, data = d) + geom_smooth()
 
 test_that("geom_smooth() respects group aesthetic", {
-  # 1 trace for points
-  # 5 traces for lines (1 for each group)
-  # 5 traces for ribbons (1 for each group)
-  info <- expect_traces(p3, 11, "group")
+  info <- expect_traces(p3, 3, "group")
 })
 
 p4 <- qplot(carat, price, colour = cut, data = d) + geom_smooth()

From 9b1ad3cdec9fcc70f505ba8b49815c5157a7cd55 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Wed, 18 Mar 2015 18:16:23 -0500
Subject: [PATCH 05/12] grr. now tests should work

---
 tests/testthat/test-ggplot-ribbon.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-ggplot-ribbon.R b/tests/testthat/test-ggplot-ribbon.R
index 736eaa9fb7..7ec354a8aa 100644
--- a/tests/testthat/test-ggplot-ribbon.R
+++ b/tests/testthat/test-ggplot-ribbon.R
@@ -26,7 +26,7 @@ p1 <- ggplot(data = huron) +
 test_that("geom_ribbon() creates 1 trace & respects alpha transparency", {
   info <- expect_traces(p1, 1, "alpha")
   tr <- info$traces[[1]]
-  expect_match(L2[[1]]$fillcolor, "0.1)", fixed=TRUE)
+  expect_match(tr$fillcolor, "0.1)", fixed=TRUE)
 })
 
 p2 <- ggplot(data = huron, aes(group = factor(decade))) + 

From 39b254657b53c4d5fba34d07c6dde38db64c3ded Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Wed, 18 Mar 2015 18:29:35 -0500
Subject: [PATCH 06/12] set default colour if no colour aes exists

---
 R/trace_generation.R | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/R/trace_generation.R b/R/trace_generation.R
index 5ef45aaf11..06b5728dfc 100644
--- a/R/trace_generation.R
+++ b/R/trace_generation.R
@@ -412,7 +412,8 @@ toBasic <- list(
     g
   },
   smoothLine=function(g) {
-    if (length(unique(g$data$group)) == 1) g$params$colour <- "#3366FF"
+    if (length(grep("^colour$", names(g$data))) == 0) 
+      g$params$colour <- "#3366FF"
     group2NA(g, "path")
   },
   smoothRibbon=function(g) {

From b50a96952d482c3322a235928970cb424c986ed1 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Thu, 19 Mar 2015 14:59:54 -0500
Subject: [PATCH 07/12] naive approach to avoiding redundant legends

---
 R/ggplotly.R                        | 29 +++++++++++++++++++++++++++--
 tests/testthat/test-ggplot-smooth.R | 28 ++++++++++++++++++++++++++--
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index 08bf4af08f..e861a71abd 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -598,13 +598,36 @@ gg2list <- function(p){
       layout$showlegend <- FALSE
     }
   }
-
+  
+  # avoid redundant legends
+  fills <- lapply(trace.list, function(x) paste0(x$name, "-", x$fillcolor))
+  linez <- lapply(trace.list, function(x) paste0(x$name, "-", x$line$color))
+  marks <- lapply(trace.list, function(x) paste0(x$name, "-", x$marker$color))
+  fill_set <- unlist(fills)
+  line_set <- unlist(linez)
+  mark_set <- unlist(marks)
+  legend_intersect <- function(x, y) {
+    i <- intersect(x, y)
+    # restrict intersection to valid legend entries
+    i[grepl("-rgb[a]?\\(", i)]
+  }
+  # if there is a mark & line legend, get rid of line
+  t1 <- line_set %in% legend_intersect(mark_set, line_set)
+  # if there is a mark & fill legend, get rid of fill
+  t2 <- fill_set %in% legend_intersect(mark_set, fill_set)
+  # if there is a line & fill legend, get rid of fill
+  t3 <- fill_set %in% legend_intersect(line_set, fill_set)
+  t <- t1 | t2 | t3
+  for (m in seq_along(trace.list)) 
+    if (trace.list[[m]]$showlegend && t[m]) 
+      trace.list[[m]]$showlegend <- FALSE
+  
   # Only show a legend title if there is at least 1 trace with
   # showlegend=TRUE.
   trace.showlegend <- sapply(trace.list, "[[", "showlegend")
   if (any(trace.showlegend) && layout$showlegend && length(p$data)) {
     # Retrieve legend title
-    legend.elements <- sapply(traces, "[[", "name")
+    legend.elements <- unlist(sapply(traces, "[[", "name"))
     legend.title <- ""
     for (i in 1:ncol(p$data)) {
       if (all(legend.elements %in% unique(p$data[, i])))
@@ -629,6 +652,8 @@ gg2list <- function(p){
     layout$annotations <- annotations
   }
   
+  
+  
   # Family font for text
   if (!is.null(theme.pars$text$family)) {
     layout$titlefont$family   <- theme.pars$text$family
diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index 04e96e8d5a..0465beef6a 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -36,13 +36,37 @@ test_that("geom_smooth() respects group aesthetic", {
 })
 
 p4 <- qplot(carat, price, colour = cut, data = d) + geom_smooth()
+p5 <- qplot(carat, price, data = d) + geom_smooth(aes(colour = cut))
 
 test_that("geom_smooth() respects colour aesthetic", {
   info <- expect_traces(p4, 11, "colour")
+  # number of showlegends should equal the number of factor levels 
+  n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
+  expect_equal(n, nlevels(d$cut))
+  info <- expect_traces(p5, 11, "colour2")
+  n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
+  expect_equal(n, nlevels(d$cut))
 })
 
-p5 <- qplot(carat, price, fill = cut, data = d) + geom_smooth()
+# why are 5 traces for point being created here??
+#p6 <- qplot(carat, price, fill = cut, data = d) + geom_smooth()
+p7 <- qplot(carat, price, data = d) + geom_smooth(aes(fill = cut))
 
 test_that("geom_smooth() respects fill aesthetic", {
-  info <- expect_traces(p5, 11, "fill")
+#   info <- expect_traces(p6, 11, "fill")
+#   n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
+#   expect_equal(n, nlevels(d$cut))
+  info <- expect_traces(p7, 11, "fill2")
+  n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
+  expect_equal(n, nlevels(d$cut))
 })
+
+# ensure legend is drawn when needed
+p8 <- qplot(carat, price, data = d) + facet_wrap(~cut) + 
+  geom_smooth(aes(colour = cut, fill = cut))
+
+test_that("geom_smooth() works with facets", {
+  # 3 traces for each panel
+  info <- expect_traces(p8, 15, "fill2")
+})
+

From 154b7790508a5da192297f191a10e2c94fb337f7 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Thu, 19 Mar 2015 16:04:35 -0500
Subject: [PATCH 08/12] Not that many traces

---
 tests/testthat/test-ggplot-smooth.R | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index 0465beef6a..e6521773c7 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -43,20 +43,15 @@ test_that("geom_smooth() respects colour aesthetic", {
   # number of showlegends should equal the number of factor levels 
   n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
   expect_equal(n, nlevels(d$cut))
-  info <- expect_traces(p5, 11, "colour2")
+  info <- expect_traces(p5, 7, "colour2")
   n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
   expect_equal(n, nlevels(d$cut))
 })
 
-# why are 5 traces for point being created here??
-#p6 <- qplot(carat, price, fill = cut, data = d) + geom_smooth()
 p7 <- qplot(carat, price, data = d) + geom_smooth(aes(fill = cut))
 
 test_that("geom_smooth() respects fill aesthetic", {
-#   info <- expect_traces(p6, 11, "fill")
-#   n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
-#   expect_equal(n, nlevels(d$cut))
-  info <- expect_traces(p7, 11, "fill2")
+  info <- expect_traces(p7, 7, "fill2")
   n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
   expect_equal(n, nlevels(d$cut))
 })

From aaf302ed8f9fb55b52f90b156425cb9f1124f52e Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Thu, 19 Mar 2015 16:54:20 -0500
Subject: [PATCH 09/12] fix typo

---
 tests/testthat/test-ggplot-smooth.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index e6521773c7..ccc87ae282 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -62,6 +62,6 @@ p8 <- qplot(carat, price, data = d) + facet_wrap(~cut) +
 
 test_that("geom_smooth() works with facets", {
   # 3 traces for each panel
-  info <- expect_traces(p8, 15, "fill2")
+  info <- expect_traces(p8, 15, "facet")
 })
 

From 2e26cc0e393426fd66c5ae47440c4e1657c343ec Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Thu, 19 Mar 2015 21:49:58 -0500
Subject: [PATCH 10/12] Avoid redunant legends entries AFTER merging traces

---
 R/ggplotly.R | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index e861a71abd..cc9eb50a6a 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -599,29 +599,6 @@ gg2list <- function(p){
     }
   }
   
-  # avoid redundant legends
-  fills <- lapply(trace.list, function(x) paste0(x$name, "-", x$fillcolor))
-  linez <- lapply(trace.list, function(x) paste0(x$name, "-", x$line$color))
-  marks <- lapply(trace.list, function(x) paste0(x$name, "-", x$marker$color))
-  fill_set <- unlist(fills)
-  line_set <- unlist(linez)
-  mark_set <- unlist(marks)
-  legend_intersect <- function(x, y) {
-    i <- intersect(x, y)
-    # restrict intersection to valid legend entries
-    i[grepl("-rgb[a]?\\(", i)]
-  }
-  # if there is a mark & line legend, get rid of line
-  t1 <- line_set %in% legend_intersect(mark_set, line_set)
-  # if there is a mark & fill legend, get rid of fill
-  t2 <- fill_set %in% legend_intersect(mark_set, fill_set)
-  # if there is a line & fill legend, get rid of fill
-  t3 <- fill_set %in% legend_intersect(line_set, fill_set)
-  t <- t1 | t2 | t3
-  for (m in seq_along(trace.list)) 
-    if (trace.list[[m]]$showlegend && t[m]) 
-      trace.list[[m]]$showlegend <- FALSE
-  
   # Only show a legend title if there is at least 1 trace with
   # showlegend=TRUE.
   trace.showlegend <- sapply(trace.list, "[[", "showlegend")
@@ -652,8 +629,6 @@ gg2list <- function(p){
     layout$annotations <- annotations
   }
   
-  
-  
   # Family font for text
   if (!is.null(theme.pars$text$family)) {
     layout$titlefont$family   <- theme.pars$text$family
@@ -785,6 +760,31 @@ gg2list <- function(p){
     merged.traces[[length(merged.traces)+1]] <- tr
   }
   
+  # avoid redundant legends entries
+  fills <- lapply(merged.traces, function(x) paste0(x$name, "-", x$fillcolor))
+  linez <- lapply(merged.traces, function(x) paste0(x$name, "-", x$line$color))
+  marks <- lapply(merged.traces, function(x) paste0(x$name, "-",x$marker$color))
+  fill_set <- unlist(fills)
+  line_set <- unlist(linez)
+  mark_set <- unlist(marks)
+  legend_intersect <- function(x, y) {
+    i <- intersect(x, y)
+    # restrict intersection to valid legend entries
+    i[grepl("-rgb[a]?\\(", i)]
+  }
+  # if there is a mark & line legend, get rid of line
+  t1 <- line_set %in% legend_intersect(mark_set, line_set)
+  # that is, unless the mode is 'lines+markers'...
+  t1 <- t1 & !(unlist(lapply(merged.traces, "[[", "mode")) %in% "lines+markers")
+  # if there is a mark & fill legend, get rid of fill
+  t2 <- fill_set %in% legend_intersect(mark_set, fill_set)
+  # if there is a line & fill legend, get rid of fill
+  t3 <- fill_set %in% legend_intersect(line_set, fill_set)
+  t <- t1 | t2 | t3
+  for (m in seq_along(merged.traces)) 
+    if (isTRUE(merged.traces[[m]]$showlegend && t[m]))
+      merged.traces[[m]]$showlegend <- FALSE
+  
   # Put the traces in correct order, according to any manually
   # specified scales. This seems to be repetitive with the trace$rank
   # attribute in layer2traces (which is useful for sorting traces that

From 18192a53c4a191b559d7ef6c606e819a930d9d15 Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Fri, 20 Mar 2015 10:58:45 -0500
Subject: [PATCH 11/12] ignore alpha when comparing legend entries

---
 R/ggplotly.R | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/R/ggplotly.R b/R/ggplotly.R
index cc9eb50a6a..798171bbde 100644
--- a/R/ggplotly.R
+++ b/R/ggplotly.R
@@ -760,13 +760,28 @@ gg2list <- function(p){
     merged.traces[[length(merged.traces)+1]] <- tr
   }
   
+  # -------------------------------
   # avoid redundant legends entries
-  fills <- lapply(merged.traces, function(x) paste0(x$name, "-", x$fillcolor))
-  linez <- lapply(merged.traces, function(x) paste0(x$name, "-", x$line$color))
-  marks <- lapply(merged.traces, function(x) paste0(x$name, "-",x$marker$color))
-  fill_set <- unlist(fills)
-  line_set <- unlist(linez)
-  mark_set <- unlist(marks)
+  # -------------------------------
+  # remove alpha from a color entry
+  rm_alpha <- function(x) {
+    if (length(x) == 0) return(x)
+    pat <- "^rgba\\("
+    if (!grepl(pat, x)) return(x)
+    sub(",\\s*[0]?[.]?[0-9]+\\)$", ")", sub(pat, "rgb(", x))
+  }
+  # convenient for extracting name/value of legend entries (ignoring alpha)
+  entries <- function(x, y) {
+    z <- try(x[[y]], silent = TRUE)
+    if (inherits(e, "try-error")) {
+      paste0(x$name, "-")
+    } else {
+      paste0(x$name, "-", rm_alpha(z))
+    }
+  }
+  fill_set <- unlist(lapply(merged.traces, entries, "fillcolor"))
+  line_set <- unlist(lapply(merged.traces, entries, c("line", "color")))
+  mark_set <- unlist(lapply(merged.traces, entries, c("marker", "color")))
   legend_intersect <- function(x, y) {
     i <- intersect(x, y)
     # restrict intersection to valid legend entries

From a13e92794153452ae16252c6799dba7983462aae Mon Sep 17 00:00:00 2001
From: cpsievert <cpsievert1@gmail.com>
Date: Fri, 20 Mar 2015 12:32:22 -0500
Subject: [PATCH 12/12] Add test for number of legend entries

---
 tests/testthat/test-ggplot-smooth.R | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/testthat/test-ggplot-smooth.R b/tests/testthat/test-ggplot-smooth.R
index ccc87ae282..cc1b9064bf 100644
--- a/tests/testthat/test-ggplot-smooth.R
+++ b/tests/testthat/test-ggplot-smooth.R
@@ -63,5 +63,7 @@ p8 <- qplot(carat, price, data = d) + facet_wrap(~cut) +
 test_that("geom_smooth() works with facets", {
   # 3 traces for each panel
   info <- expect_traces(p8, 15, "facet")
+  n <- sum(unlist(sapply(info$traces, "[[", "showlegend")))
+  expect_equal(n, nlevels(d$cut))
 })