grouped mutate promotes up results that consists of all NA in one g…

…roup. closes #1463.
tidyverse · Oct 28, 2015 · 013338a · 013338a
1 parent 67d1ed0
commit 013338a
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 30 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -42,6 +42,8 @@
    This is useful to disambiguate the case where `var` is also a variable from
    the dataset (#1469).
 
+* grouped `mutate` promotes up results that consists of all NA in one group (#1463). 
+
 # dplyr 0.4.3
 
 ## Improved encoding support

diff --git a/inst/include/dplyr/Gatherer.h b/inst/include/dplyr/Gatherer.h
@@ -39,15 +39,18 @@ namespace dplyr {
 
         inline void grab(SEXP data, const SlicingIndex& indices){
             int n = Rf_length(data) ;
-
-            check_type(data) ;
-            if(n == indices.size() ){
-                grab_along( data, indices ) ;
-            } else if( n == 1) {
-                grab_rep( Rcpp::internal::r_vector_start<RTYPE>(data)[0], indices ) ;
+            if( is<LogicalVector>(data) && all(is_na(LogicalVector(data))).is_true() ){
+              grab_rep( Vector<RTYPE>::get_na(), indices ) ;
             } else {
-                stop ( "incompatible size (%d), expecting %d (the group size) or 1",
-                        n, indices.size()) ;
+              check_type(data) ;
+              if(n == indices.size() ){
+                  grab_along( data, indices ) ;
+              } else if( n == 1) {
+                  grab_rep( Rcpp::internal::r_vector_start<RTYPE>(data)[0], indices ) ;
+              } else {
+                  stop ( "incompatible size (%d), expecting %d (the group size) or 1",
+                          n, indices.size()) ;
+              }
             }
         }
 

diff --git a/tests/testthat/test-mutate.r b/tests/testthat/test-mutate.r
@@ -440,26 +440,32 @@ test_that("mutate handles factors (#1414)", {
 })
 
 test_that("mutate recognizes global #1469", {
-    vs <- 4
-    res <- mtcars %>% mutate(a = global(vs))
-    expect_true( all(res$a == 4) )
-    expect_error( mtcars %>% mutate(global("vs")), "global only handles symbols" )
-    res <- mtcars %>% mutate(a = global(vs) + 1)
-    expect_true( all(res$a == 5) )
-    expect_error( mtcars %>% mutate(global("vs") + 1), "global only handles symbols" )
-    res <- mtcars %>% mutate(a = 1+global(vs) )
-    expect_true( all(res$a == 5) )
-    expect_error( mtcars %>% mutate(1 + global("vs")), "global only handles symbols" )
-
-    res <- mtcars %>% group_by(cyl) %>% mutate(a = global(vs))
-    expect_true( all(res$a == 4) )
-    expect_error( mtcars %>% group_by(cyl) %>% mutate(a = global("vs")), "global only handles symbols" )
-    res <- mtcars %>% group_by(cyl) %>% mutate(a = global(vs)+1)
-    expect_true( all(res$a == 5) )
-    expect_error( mtcars %>% group_by(cyl) %>% mutate(a = global("vs") + 1), "global only handles symbols" )
-
-    res <- mtcars %>% group_by(cyl) %>% mutate(a = 1+global(vs))
-    expect_true( all(res$a == 5) )
-    expect_error( mtcars %>% group_by(cyl) %>% mutate(a = 1 + global("vs")), "global only handles symbols" )
-
+  vs <- 4
+  res <- mtcars %>% mutate(a = global(vs))
+  expect_true( all(res$a == 4) )
+  expect_error( mtcars %>% mutate(global("vs")), "global only handles symbols" )
+  res <- mtcars %>% mutate(a = global(vs) + 1)
+  expect_true( all(res$a == 5) )
+  expect_error( mtcars %>% mutate(global("vs") + 1), "global only handles symbols" )
+  res <- mtcars %>% mutate(a = 1+global(vs) )
+  expect_true( all(res$a == 5) )
+  expect_error( mtcars %>% mutate(1 + global("vs")), "global only handles symbols" )
+
+  res <- mtcars %>% group_by(cyl) %>% mutate(a = global(vs))
+  expect_true( all(res$a == 4) )
+  expect_error( mtcars %>% group_by(cyl) %>% mutate(a = global("vs")), "global only handles symbols" )
+  res <- mtcars %>% group_by(cyl) %>% mutate(a = global(vs)+1)
+  expect_true( all(res$a == 5) )
+  expect_error( mtcars %>% group_by(cyl) %>% mutate(a = global("vs") + 1), "global only handles symbols" )
+
+  res <- mtcars %>% group_by(cyl) %>% mutate(a = 1+global(vs))
+  expect_true( all(res$a == 5) )
+  expect_error( mtcars %>% group_by(cyl) %>% mutate(a = 1 + global("vs")), "global only handles symbols" )
+})
+
+test_that("mutate handles results from one group with all NA values (#1463) ", {
+  df <- data_frame( x = c(1, 2), y = c(1, NA))
+  res <- df %>% group_by(x) %>% mutate( z = ifelse(y>1, 1, 2) )
+  expect_true( is.na(res$z[2]) )
+  expect_is( res$z, "numeric")
 })