From 060ebdcec192100d7471a9d133ffd1448d6f07fd Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Fri, 23 Jun 2017 13:34:45 -0700
Subject: [PATCH 1/8] Add scale_size_ordinal

---
 NAMESPACE         |  1 +
 R/scale-size.r    | 10 ++++++++++
 man/scale_size.Rd |  1 +
 3 files changed, 12 insertions(+)

diff --git a/NAMESPACE b/NAMESPACE
index a8dc92682c..de19ed9550 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -424,6 +424,7 @@ export(scale_size_datetime)
 export(scale_size_discrete)
 export(scale_size_identity)
 export(scale_size_manual)
+export(scale_size_ordinal)
 export(scale_type)
 export(scale_x_continuous)
 export(scale_x_date)
diff --git a/R/scale-size.r b/R/scale-size.r
index 439391dc59..0a9b073cf1 100644
--- a/R/scale-size.r
+++ b/R/scale-size.r
@@ -67,6 +67,16 @@ scale_size_discrete <- function(..., range = c(2, 6)) {
   }, ...)
 }
 
+#' @rdname scale_size
+#' @export
+#' @usage NULL
+scale_size_ordinal <- function(..., range = c(2, 6)) {
+  discrete_scale("size", "size_d", function(n) {
+    area <- seq(range[1] ^ 2, range[2] ^ 2, length.out = n)
+    sqrt(area)
+  }, ...)
+}
+
 #' @inheritDotParams continuous_scale -aesthetics -scale_name -palette -rescaler
 #' @param max_size Size of largest points.
 #' @export
diff --git a/man/scale_size.Rd b/man/scale_size.Rd
index df65a70996..92f9518b36 100644
--- a/man/scale_size.Rd
+++ b/man/scale_size.Rd
@@ -6,6 +6,7 @@
 \alias{scale_radius}
 \alias{scale_size}
 \alias{scale_size_discrete}
+\alias{scale_size_ordinal}
 \alias{scale_size_area}
 \alias{scale_size_datetime}
 \alias{scale_size_date}

From 5da34ae6928ec9024f3f2cb4c2a0226deb7fb43a Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Fri, 23 Jun 2017 13:35:30 -0700
Subject: [PATCH 2/8] Add scale_shape_ordinal, which throws a warning when
 mapping shape to an ordered factor

---
 NAMESPACE          | 1 +
 R/scale-shape.r    | 8 ++++++++
 man/scale_shape.Rd | 1 +
 3 files changed, 10 insertions(+)

diff --git a/NAMESPACE b/NAMESPACE
index de19ed9550..fb167152fc 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -416,6 +416,7 @@ export(scale_shape_continuous)
 export(scale_shape_discrete)
 export(scale_shape_identity)
 export(scale_shape_manual)
+export(scale_shape_ordinal)
 export(scale_size)
 export(scale_size_area)
 export(scale_size_continuous)
diff --git a/R/scale-shape.r b/R/scale-shape.r
index fd064088e7..fd4734a9e1 100644
--- a/R/scale-shape.r
+++ b/R/scale-shape.r
@@ -43,6 +43,14 @@ scale_shape <- function(..., solid = TRUE) {
 #' @usage NULL
 scale_shape_discrete <- scale_shape
 
+#' @rdname scale_shape
+#' @export
+#' @usage NULL
+scale_shape_ordinal <- scale_shape <- function(..., solid = TRUE) {
+  warning("Using shapes for an ordinal variable is not advised", call. = FALSE)
+  discrete_scale("shape", "shape_d", shape_pal(solid), ...)
+}
+
 #' @rdname scale_shape
 #' @export
 #' @usage NULL
diff --git a/man/scale_shape.Rd b/man/scale_shape.Rd
index f25f2afa11..cb0692bcaf 100644
--- a/man/scale_shape.Rd
+++ b/man/scale_shape.Rd
@@ -3,6 +3,7 @@
 \name{scale_shape}
 \alias{scale_shape}
 \alias{scale_shape_discrete}
+\alias{scale_shape_ordinal}
 \alias{scale_shape_continuous}
 \title{Scales for shapes, aka glyphs}
 \usage{

From e9670b417f9cc28094a518c3458c7e29b2a69ed4 Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Fri, 23 Jun 2017 14:46:48 -0700
Subject: [PATCH 3/8] Warn when using alpha for factors (but not ordered
 factors)

---
 NAMESPACE          | 1 +
 R/scale-alpha.r    | 8 ++++++++
 man/scale_alpha.Rd | 3 +++
 3 files changed, 12 insertions(+)

diff --git a/NAMESPACE b/NAMESPACE
index fb167152fc..f8fb833c7d 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -368,6 +368,7 @@ export(scale_alpha_continuous)
 export(scale_alpha_discrete)
 export(scale_alpha_identity)
 export(scale_alpha_manual)
+export(scale_alpha_ordinal)
 export(scale_color_brewer)
 export(scale_color_continuous)
 export(scale_color_discrete)
diff --git a/R/scale-alpha.r b/R/scale-alpha.r
index 522266974b..4dd8cc42b2 100644
--- a/R/scale-alpha.r
+++ b/R/scale-alpha.r
@@ -29,6 +29,14 @@ scale_alpha_continuous <- scale_alpha
 #' @rdname scale_alpha
 #' @export
 scale_alpha_discrete <- function(..., range = c(0.1, 1)) {
+  warning("Using alpha for a discrete variable is not advised.", call. = FALSE)
+  discrete_scale("alpha", "alpha_d",
+    function(n) seq(range[1], range[2], length.out = n), ...)
+}
+
+#' @rdname scale_alpha
+#' @export
+scale_alpha_ordinal <- function(..., range = c(0.1, 1)) {
   discrete_scale("alpha", "alpha_d",
     function(n) seq(range[1], range[2], length.out = n), ...)
 }
diff --git a/man/scale_alpha.Rd b/man/scale_alpha.Rd
index d51e5e863e..53b5fcea33 100644
--- a/man/scale_alpha.Rd
+++ b/man/scale_alpha.Rd
@@ -4,6 +4,7 @@
 \alias{scale_alpha}
 \alias{scale_alpha_continuous}
 \alias{scale_alpha_discrete}
+\alias{scale_alpha_ordinal}
 \title{Alpha transparency scales}
 \usage{
 scale_alpha(..., range = c(0.1, 1))
@@ -11,6 +12,8 @@ scale_alpha(..., range = c(0.1, 1))
 scale_alpha_continuous(..., range = c(0.1, 1))
 
 scale_alpha_discrete(..., range = c(0.1, 1))
+
+scale_alpha_ordinal(..., range = c(0.1, 1))
 }
 \arguments{
 \item{...}{Other arguments passed on to \code{\link[=continuous_scale]{continuous_scale()}}

From 1b5e3e8a3eee2c0b21307da1964499ca15672daf Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Fri, 23 Jun 2017 16:36:34 -0700
Subject: [PATCH 4/8] Add a line to NEWS.md

---
 NEWS.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/NEWS.md b/NEWS.md
index 91301ef55d..7f363125db 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,10 @@
 # ggplot2 2.2.1.9000
 
+* Ordered factors now behave differently from unordered factors in some cases.
+  Ordered factors throw a warning when mapped to shape (unordered factors do
+  not). Ordered factors do not throw warnings when mapped to size or alpha
+  (unordered factors do) (@karawoo, #1526).
+
 * Use `rel()` to set line widths in theme defaults (@baptiste).
 
 * `geom_density` drops groups with fewer than two data points and throws a

From 14233b897788f6c6d88bc8eac4dbd68f35fb883b Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Wed, 28 Jun 2017 14:10:50 -0700
Subject: [PATCH 5/8] DRY up alpha, shape, and size

---
 R/scale-alpha.r    | 13 ++++++++-----
 R/scale-shape.r    |  4 ++--
 R/scale-size.r     | 20 +++++++++++---------
 man/scale_alpha.Rd |  2 +-
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/R/scale-alpha.r b/R/scale-alpha.r
index 4dd8cc42b2..4ac54967b2 100644
--- a/R/scale-alpha.r
+++ b/R/scale-alpha.r
@@ -28,15 +28,18 @@ scale_alpha_continuous <- scale_alpha
 
 #' @rdname scale_alpha
 #' @export
-scale_alpha_discrete <- function(..., range = c(0.1, 1)) {
+scale_alpha_discrete <- function(...) {
   warning("Using alpha for a discrete variable is not advised.", call. = FALSE)
-  discrete_scale("alpha", "alpha_d",
-    function(n) seq(range[1], range[2], length.out = n), ...)
+  scale_alpha_ordinal(...)
 }
 
 #' @rdname scale_alpha
 #' @export
 scale_alpha_ordinal <- function(..., range = c(0.1, 1)) {
-  discrete_scale("alpha", "alpha_d",
-    function(n) seq(range[1], range[2], length.out = n), ...)
+  discrete_scale(
+    "alpha",
+    "alpha_d",
+    function(n) seq(range[1], range[2], length.out = n),
+    ...
+  )
 }
diff --git a/R/scale-shape.r b/R/scale-shape.r
index fd4734a9e1..2a7c8cabac 100644
--- a/R/scale-shape.r
+++ b/R/scale-shape.r
@@ -46,9 +46,9 @@ scale_shape_discrete <- scale_shape
 #' @rdname scale_shape
 #' @export
 #' @usage NULL
-scale_shape_ordinal <- scale_shape <- function(..., solid = TRUE) {
+scale_shape_ordinal <- function(...) {
   warning("Using shapes for an ordinal variable is not advised", call. = FALSE)
-  discrete_scale("shape", "shape_d", shape_pal(solid), ...)
+  scale_shape(...)
 }
 
 #' @rdname scale_shape
diff --git a/R/scale-size.r b/R/scale-size.r
index 0a9b073cf1..fe2488f5fc 100644
--- a/R/scale-size.r
+++ b/R/scale-size.r
@@ -59,22 +59,24 @@ scale_size <- scale_size_continuous
 #' @rdname scale_size
 #' @export
 #' @usage NULL
-scale_size_discrete <- function(..., range = c(2, 6)) {
+scale_size_discrete <- function(...) {
   warning("Using size for a discrete variable is not advised.", call. = FALSE)
-  discrete_scale("size", "size_d", function(n) {
-    area <- seq(range[1] ^ 2, range[2] ^ 2, length.out = n)
-    sqrt(area)
-  }, ...)
+  scale_size_ordinal(...)
 }
 
 #' @rdname scale_size
 #' @export
 #' @usage NULL
 scale_size_ordinal <- function(..., range = c(2, 6)) {
-  discrete_scale("size", "size_d", function(n) {
-    area <- seq(range[1] ^ 2, range[2] ^ 2, length.out = n)
-    sqrt(area)
-  }, ...)
+  discrete_scale(
+    "size",
+    "size_d",
+    function(n) {
+      area <- seq(range[1] ^ 2, range[2] ^ 2, length.out = n)
+      sqrt(area)
+    },
+    ...
+  )
 }
 
 #' @inheritDotParams continuous_scale -aesthetics -scale_name -palette -rescaler
diff --git a/man/scale_alpha.Rd b/man/scale_alpha.Rd
index 53b5fcea33..55e9ee9684 100644
--- a/man/scale_alpha.Rd
+++ b/man/scale_alpha.Rd
@@ -11,7 +11,7 @@ scale_alpha(..., range = c(0.1, 1))
 
 scale_alpha_continuous(..., range = c(0.1, 1))
 
-scale_alpha_discrete(..., range = c(0.1, 1))
+scale_alpha_discrete(...)
 
 scale_alpha_ordinal(..., range = c(0.1, 1))
 }

From 6f6e2b2143fe3745ad916acc9420a55ab80cb001 Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Wed, 28 Jun 2017 14:44:36 -0700
Subject: [PATCH 6/8] Add tests for size/alpha/shape behavior with factors

---
 tests/testthat/test-scales.r | 42 ++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/testthat/test-scales.r b/tests/testthat/test-scales.r
index db52d3aee7..947a6f1f66 100644
--- a/tests/testthat/test-scales.r
+++ b/tests/testthat/test-scales.r
@@ -188,3 +188,45 @@ test_that("Scales get their correct titles through layout", {
   expect_identical(p$layout$xlabel(p$plot$labels)$primary, "x")
   expect_identical(p$layout$ylabel(p$plot$labels)$primary, "y")
 })
+
+test_that("Size and alpha scales throw appropriate warnings for factors", {
+  df <- data.frame(
+    x = 1:3,
+    y = 1:3,
+    d = LETTERS[1:3],
+    o = factor(LETTERS[1:3], ordered = TRUE)
+  )
+  p <- ggplot(df, aes(x, y))
+
+  # There should be warnings when unordered factors are mapped to size/alpha
+  expect_warning(
+    ggplot_build(p + geom_point(aes(size = d))),
+    "Using size for a discrete variable is not advised."
+  )
+  expect_warning(
+    ggplot_build(p + geom_point(aes(alpha = d))),
+    "Using alpha for a discrete variable is not advised."
+  )
+  # There should be no warnings for ordered factors
+  expect_warning(ggplot_build(p + geom_point(aes(size = o))), NA)
+  expect_warning(ggplot_build(p + geom_point(aes(alpha = o))), NA) 
+})
+
+test_that("Shape scale throws appropriate warnings for factors", {
+  df <- data.frame(
+    x = 1:3,
+    y = 1:3,
+    d = LETTERS[1:3],
+    o = factor(LETTERS[1:3], ordered = TRUE)
+  )
+  p <- ggplot(df, aes(x, y))
+
+  # There should be no warnings when unordered factors are mapped to shape
+  expect_warning(ggplot_build(p + geom_point(aes(shape = d))), NA)
+
+  # There should be warnings for ordered factors
+  expect_warning(
+    ggplot_build(p + geom_point(aes(shape = o))),
+    "Using shapes for an ordinal variable is not advised"
+  )
+})

From b9e8819ddcdd64c2cdadad197ec72064636db449 Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Fri, 14 Jul 2017 11:31:22 -0700
Subject: [PATCH 7/8] Use viridis as default for ordered factors

---
 NAMESPACE                     |  2 ++
 NEWS.md                       |  3 ++-
 R/zxx.r                       | 10 ++++++++++
 man/scale_viridis.Rd          |  2 ++
 tests/testthat/test-viridis.R |  8 +++++++-
 5 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/NAMESPACE b/NAMESPACE
index 1e0cc68be9..14848027f4 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -401,6 +401,7 @@ export(scale_colour_grey)
 export(scale_colour_hue)
 export(scale_colour_identity)
 export(scale_colour_manual)
+export(scale_colour_ordinal)
 export(scale_colour_viridis_c)
 export(scale_colour_viridis_d)
 export(scale_fill_brewer)
@@ -416,6 +417,7 @@ export(scale_fill_grey)
 export(scale_fill_hue)
 export(scale_fill_identity)
 export(scale_fill_manual)
+export(scale_fill_ordinal)
 export(scale_fill_viridis_c)
 export(scale_fill_viridis_d)
 export(scale_linetype)
diff --git a/NEWS.md b/NEWS.md
index 8991370ee0..faf068d0ac 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -3,7 +3,8 @@
 * Ordered factors now behave differently from unordered factors in some cases.
   Ordered factors throw a warning when mapped to shape (unordered factors do
   not). Ordered factors do not throw warnings when mapped to size or alpha
-  (unordered factors do) (@karawoo, #1526).
+  (unordered factors do). Viridis is the default colour and fill scale for
+  ordered factors (@karawoo, #1526).
 
 * The `show.legend` parameter now accepts a named logical vector to hide/show
   only some aesthetics in the legend (@tutuchan, #1798)
diff --git a/R/zxx.r b/R/zxx.r
index 1728dbd0d9..9624526b90 100644
--- a/R/zxx.r
+++ b/R/zxx.r
@@ -5,6 +5,11 @@
 #' @usage NULL
 scale_colour_discrete <- scale_colour_hue
 
+#' @export
+#' @rdname scale_viridis
+#' @usage NULL
+scale_colour_ordinal <- scale_colour_viridis_d
+
 #' @export
 #' @rdname scale_gradient
 #' @usage NULL
@@ -48,6 +53,11 @@ scale_colour_date <- function(...,
 #' @usage NULL
 scale_fill_discrete <- scale_fill_hue
 
+#' @export
+#' @rdname scale_viridis
+#' @usage NULL
+scale_fill_ordinal <- scale_fill_viridis_d
+
 #' @export
 #' @rdname scale_gradient
 #' @usage NULL
diff --git a/man/scale_viridis.Rd b/man/scale_viridis.Rd
index 4907154963..3ece88770f 100644
--- a/man/scale_viridis.Rd
+++ b/man/scale_viridis.Rd
@@ -5,6 +5,8 @@
 \alias{scale_fill_viridis_d}
 \alias{scale_colour_viridis_c}
 \alias{scale_fill_viridis_c}
+\alias{scale_colour_ordinal}
+\alias{scale_fill_ordinal}
 \alias{scale_color_viridis_d}
 \alias{scale_color_viridis_c}
 \title{Viridis colour scales from viridisLite}
diff --git a/tests/testthat/test-viridis.R b/tests/testthat/test-viridis.R
index d1531c1e40..242a99dfc3 100644
--- a/tests/testthat/test-viridis.R
+++ b/tests/testthat/test-viridis.R
@@ -1,6 +1,6 @@
 context("Viridis")
 
-df <- data.frame(x = 1, y = 1, z = "a")
+df <- data.frame(x = 1, y = 1, z = "a", tier = factor("low", ordered = TRUE))
 
 test_that("Viridis scale changes point color", {
   p1 <- ggplot(df, aes(x, y, colour = z)) +
@@ -10,3 +10,9 @@ test_that("Viridis scale changes point color", {
   expect_false(layer_data(p1)$colour == layer_data(p2)$colour)
   expect_equal(layer_data(p2)$colour, "#440154FF")
 })
+
+test_that("Viridis scale is used by default for ordered factors", {
+  p <- ggplot(df, aes(x, y, colour = tier)) + geom_point()
+  
+  expect_equal(layer_data(p)$colour, "#440154FF")
+})

From 74476719b3d5305afa1042327e8ccb7ab4536dcc Mon Sep 17 00:00:00 2001
From: Kara Woo <woo.kara@gmail.com>
Date: Tue, 18 Jul 2017 08:12:09 -0700
Subject: [PATCH 8/8] Move viridisLite to Imports

---
 DESCRIPTION | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index 3c8ed117ea..31a3dc2f85 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -22,7 +22,8 @@ Imports:
     scales (>= 0.4.1.9002),
     stats,
     tibble,
-    lazyeval
+    lazyeval,
+    viridisLite
 Suggests:
     covr,
     ggplot2movies,
@@ -43,8 +44,7 @@ Suggests:
     rpart,
     rmarkdown,
     sf (>= 0.3-4),
-    svglite (>= 1.2.0.9001),
-    viridisLite
+    svglite (>= 1.2.0.9001)
 Remotes:
     hadley/scales,
     hadley/svglite