more on #1142

tidyverse · Jun 25, 2015 · fb33393 · fb33393
1 parent 9e19c1c
commit fb33393
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 18 deletions.
diff --git a/inst/include/dplyr/GroupedDataFrame.h b/inst/include/dplyr/GroupedDataFrame.h
@@ -4,9 +4,11 @@
 namespace Rcpp {
 
     inline void check_valid_colnames( const DataFrame& df){
-        CharacterVector names(df.names()) ;
-        if( any( duplicated(names) ).is_true() ){
-            stop("found duplicated column name") ;    
+        if( df.size() ){
+            CharacterVector names(df.names()) ;
+            if( any( duplicated(names) ).is_true() ){
+                stop("found duplicated column name") ;    
+            }
         }
     }
 

diff --git a/inst/include/dplyr/Result/LazySubsets.h b/inst/include/dplyr/Result/LazySubsets.h
@@ -9,13 +9,16 @@ namespace dplyr {
         typedef DataMap::const_iterator const_iterator ;
 
         LazySubsets( const DataFrame& df) : data_map(), nr(df.nrows()){
-            CharacterVector names = df.names() ;
-            for( int i=0; i<df.size(); i++){
-                SEXP column = df[i] ;
-                if( Rf_inherits( column, "matrix" ) ){
-                    stop( "matrix as column is not supported" ) ;    
+            int nvars = df.size() ;
+            if( nvars ){
+                CharacterVector names = df.names() ;
+                for( int i=0; i<nvars; i++){
+                    SEXP column = df[i] ;
+                    if( Rf_inherits( column, "matrix" ) ){
+                        stop( "matrix as column is not supported" ) ;    
+                    }
+                    data_map[as_symbol(names[i])] = df[i] ;    
                 }
-                data_map[as_symbol(names[i])] = df[i] ;    
             }
         }
         virtual ~LazySubsets(){}

diff --git a/src/dplyr.cpp b/src/dplyr.cpp
@@ -1953,12 +1953,15 @@ SEXP mutate_grouped(const DataFrame& df, const LazyDots& dots){
 
 SEXP mutate_not_grouped(DataFrame df, const LazyDots& dots){
     int nexpr = dots.size() ;
-
+    int nrows = df.nrows() ;
+
     NamedListAccumulator<DataFrame> accumulator ;
     int nvars = df.size() ;
-    CharacterVector df_names = df.names() ;
-    for( int i=0; i<nvars; i++){
-        accumulator.set( df_names[i], df[i] ) ;
+    if( nvars ){
+        CharacterVector df_names = df.names() ;
+        for( int i=0; i<nvars; i++){
+            accumulator.set( df_names[i], df[i] ) ;
+        }
     }
 
     CallProxy call_proxy(df) ;
@@ -1982,7 +1985,7 @@ SEXP mutate_not_grouped(DataFrame df, const LazyDots& dots){
             call_proxy.set_call( call );
             result = call_proxy.eval() ;
         } else if( Rf_length(call) == 1 ){
-            boost::scoped_ptr<Gatherer> gather( constant_gatherer<DataFrame,LazySubsets>( call, df.nrows() ) );
+            boost::scoped_ptr<Gatherer> gather( constant_gatherer<DataFrame,LazySubsets>( call, nrows ) );
             result = gather->collect() ;
         } else if( Rf_isNull(call)) {
             accumulator.rm(name) ;
@@ -1996,20 +1999,20 @@ SEXP mutate_not_grouped(DataFrame df, const LazyDots& dots){
         if( Rf_inherits(result, "POSIXlt") ){
             stop("`mutate` does not support `POSIXlt` results");
         }
-        if( Rf_length(result) == df.nrows() ){
+        int n_res = Rf_length(result) ;
+        if( n_res == nrows ){
             // ok
-        } else if( Rf_length(result) == 1 ){
+        } else if( n_res == 1 && nrows != 0 ){
             // recycle
             boost::scoped_ptr<Gatherer> gather( constant_gatherer<DataFrame,LazySubsets>( result, df.nrows() ) );
             result = gather->collect() ;
         } else {
-            stop( "wrong result size (%d), expected %d or 1", Rf_length(result), df.nrows() ) ;
+            stop( "wrong result size (%d), expected %d or 1", n_res, nrows ) ;
         }
 
         call_proxy.input( name, result ) ;
         accumulator.set( name, result );
     }
-
     List res = structure_mutate(accumulator, df, classes_not_grouped() ) ;
 
     return res ;

diff --git a/tests/testthat/test-mutate.r b/tests/testthat/test-mutate.r
@@ -367,4 +367,9 @@ test_that("mutate works on empty data frames (#1142)", {
   res <- df %>% mutate
   expect_equal( nrow(res), 0L )
   expect_equal( length(res), 0L )
+
+  res <- df %>% mutate(x = numeric())
+  expect_equal( names(res), "x")
+  expect_equal( nrow(res), 0L )
+  expect_equal( length(res), 1L) 
 })