remove_missing() considers characters as finite (#5188)

* consider characters as finite

* don't simplify after removing missing

* Add test

* Add NEWS bullet

* Correct mistake

Author: teunbrand

NEWS.md

@@ -1,5 +1,7 @@
 # ggplot2 (development version)
 
+* For the purposes of checking required or non-missing aesthetics, character 
+  vectors are no longer considered non-finite (@teunbrand, @4284).
 * Fixed bug in `coord_sf()` where graticule lines didn't obey 
   `panel.grid.major`'s linewidth setting (@teunbrand, #5179)
 * The `datetime_scale()` scale constructor is now exported for use in extension

R/utilities.r

@@ -90,7 +90,7 @@ remove_missing <- function(df, na.rm = FALSE, vars = names(df), name = "",
   missing <- detect_missing(df, vars, finite)
 
   if (any(missing)) {
-    df <- df[!missing, ]
+    df <- df[!missing, , drop = FALSE]
     if (!na.rm) {
       if (name != "") name <- paste(" ({.fn ", name, "})", sep = "")
       msg <- paste0(
@@ -125,10 +125,12 @@ cases <- function(x, fun) {
   }
 }
 
-# Wrapper around is.finite to handle list cols
+# Wrapper around is.finite to handle list and character cols
 is_finite <- function(x) {
   if (typeof(x) == "list") {
     !vapply(x, is.null, logical(1))
+  } else if (typeof(x) == "character") {
+    !is.na(x)
   } else {
     is.finite(x)
   }

tests/testthat/test-utilities.r

@@ -104,6 +104,14 @@ test_that("remove_missing checks input", {
   expect_snapshot_error(remove_missing(na.rm = 1:5))
 })
 
+test_that("characters survive remove_missing", {
+  data <- data_frame0(x = c("A", NA))
+  expect_warning(
+    new <- remove_missing(data, finite = TRUE)
+  )
+  expect_equal(new, data_frame0(x = "A"))
+})
+
 test_that("tolower() and toupper() has been masked", {
   expect_snapshot_error(tolower())
   expect_snapshot_error(toupper())