better error message with a suggestion to use forcats for implicit mi…

…ssing values in factor when used in group_by.

Eliminate the lazy use of the data frame from MASS 📦

src/group_indices.cpp

@@ -247,13 +247,13 @@ class FactorSlicer : public Slicer {
       int idx = range[i];
       int value = f[idx];
 
-      // will support it later
-      if (value == NA_INTEGER) stop("NA not supported");
+      if (value == NA_INTEGER) {
+        bad_col(visitors.name(depth), "contains implicit missing values. Consider `forcats::fct_explicit_na` to turn them explicit");
+      }
       indices[value - 1].push_back(idx);
     }
   }
 
-
   int depth;
 
   const std::vector<SEXP>& data;

tests/testthat/test-group-by.r

@@ -70,9 +70,8 @@ test_that("group_by uses shallow copy", {
 })
 
 test_that("FactorVisitor handles NA. #183", {
-  skip("until we can group again by a factor that has NA")
-  g <- group_by(MASS::survey, M.I)
-  expect_equal(g$M.I, MASS::survey$M.I)
+  d <- data.frame(x = 1:3, f = factor(c("a", "b", NA)))
+  expect_error(group_by(d, f), "Column `f` contains implicit missing values")
 })
 
 test_that("group_by orders by groups. #242", {