slice correctly handles grouped attributes. closes tidyverse#1405.

mikeaddison93 · Sep 12, 2015 · cc915a2 · cc915a2
1 parent e8804ae
commit cc915a2
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 9 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -8,6 +8,8 @@
 
 * `n_distinct` uses multiple arguments (#1084).
 
+* `slice` correctly handles grouped attributes (#1405).
+
 # dplyr 0.4.3
 
 ## Improved encoding support

diff --git a/inst/include/dplyr.h b/inst/include/dplyr.h
@@ -51,6 +51,7 @@ namespace dplyr {
     class LazySubsets ;
     std::string get_single_class(SEXP x) ;
 
+    void strip_index(DataFrame x) ;
     template <typename Index>
     DataFrame subset( DataFrame df, const Index& indices, CharacterVector classes) ;
 

diff --git a/src/dplyr.cpp b/src/dplyr.cpp
@@ -123,7 +123,7 @@ Result* minmax_prototype( SEXP call, const LazySubsets& subsets, int nargs ){
     } else {
       return 0 ;
     }
-    
+
     if( nargs == 1 ){
         return minmax_prototype_impl<Tmpl,false>(arg, is_summary) ;
     } else if( nargs == 2 ){
@@ -1619,8 +1619,9 @@ SEXP slice_grouped(GroupedDataFrame gdf, const LazyDots& dots){
     }
     DataFrame res = subset( data, indx, names, classes_grouped<GroupedDataFrame>() ) ;
     res.attr( "vars")   = data.attr("vars") ;
+    strip_index(res) ;
 
-    return res ;
+    return GroupedDataFrame(res).data() ;
 
 }
 

diff --git a/src/filter.cpp b/src/filter.cpp
@@ -5,11 +5,13 @@ using namespace dplyr ;
 
 typedef dplyr_hash_set<SEXP> SymbolSet ;
 
-void strip_index(DataFrame x) {
-  x.attr("indices") = R_NilValue ;
-  x.attr("group_sizes") = R_NilValue ;
-  x.attr("biggest_group_size") = R_NilValue ;
-  x.attr("labels") = R_NilValue ;
+namespace dplyr {
+  void strip_index(DataFrame x) {
+    x.attr("indices") = R_NilValue ;
+    x.attr("group_sizes") = R_NilValue ;
+    x.attr("biggest_group_size") = R_NilValue ;
+    x.attr("labels") = R_NilValue ;
+  }
 }
 
 inline SEXP empty_subset( const DataFrame& df, CharacterVector columns, CharacterVector classes ){

diff --git a/tests/testthat/test-slice.r b/tests/testthat/test-slice.r
@@ -81,12 +81,12 @@ test_that( "slice handles NA (#1235)", {
   expect_equal( nrow(slice(df, NA_integer_)), 0L )
   expect_equal( nrow(slice(df, c(1L, NA_integer_))), 1L )
   expect_equal( nrow(slice(df, c(-1L, NA_integer_))), 2L )
-  
+
   df <- data_frame( x = 1:4, g = rep(1:2, 2) ) %>% group_by(g)
   expect_equal( nrow(slice(df, NA)), 0L )
   expect_equal( nrow(slice(df, c(1,NA))), 2 )
   expect_equal( nrow(slice(df, c(-1,NA))), 2 )
-  
+
 })
 
 test_that("slice handles empty data frames (#1219)", {
@@ -101,3 +101,9 @@ test_that("slice works fine if n > nrow(df) (#1269)", {
   filter_res <- mtcars %>% group_by(cyl) %>% filter( row_number() == 8 )
   expect_equal( slice_res, filter_res )
 })
+
+test_that("slice strips grouped indices (#1405)", {
+  res <- mtcars %>% group_by(cyl) %>% slice(1) %>% mutate(mpgplus = mpg + 1)
+  expect_equal( nrow(res), 3L)
+  expect_equal( attr(res, "indices"), as.list(0:2) )
+})