Skip to content

Commit

Permalink
Merge pull request #476 from arunsrinivasan/invalid_selfref
Browse files Browse the repository at this point in the history
Fix for #475
  • Loading branch information
hadley committed Jul 30, 2014
2 parents 5a1da8c + 8ed01aa commit 6d789f0
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 7 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
* `do()` now displays the progress bar only when used in interactive prompts
and not when knitting (#428, @jimhester).

* `summarise` and `group_by` now retain over allocation when working with data.tables (#475, arunsrinivasan).

# dplyr 0.2

Expand Down
10 changes: 5 additions & 5 deletions R/grouped-dt.r
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ grouped_dt <- function(data, vars) {

data <- copy(data)
setkeyv(data, deparse_all(vars))

structure(data, vars = vars,
class = c("grouped_dt", "tbl_dt", "tbl", class(data)))
setattr(data, "vars", vars)
setattr(data, "class", c("grouped_dt", "tbl_dt", "tbl", class(data)))
data
}

#' @export
Expand Down Expand Up @@ -74,7 +74,7 @@ regroup.grouped_dt <- function(x, value) {

#' @export
ungroup.grouped_dt <- function(x) {
attr(x, "vars") <- NULL
class(x) <- setdiff(class(x), "grouped_dt")
setattr(x, "vars", NULL)
setattr(x, "class", setdiff(class(x), "grouped_dt"))
x
}
5 changes: 3 additions & 2 deletions R/tbl-dt.r
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ tbl_dt <- function(data) {
}
if (is.grouped_dt(data)) return(ungroup(data))

data <- as.data.table(data)
structure(data, class = c("tbl_dt", "tbl", "data.table", "data.frame"))
if (is.data.table(data)) data <- copy(data) else data <- as.data.table(data)
setattr(data, "class", c("tbl_dt", "tbl", "data.table", "data.frame"))
data
}

#' @export
Expand Down
7 changes: 7 additions & 0 deletions tests/testthat/test-group-by.r
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,10 @@ test_that("group_by only creates one group for NA (#401)", {
res <- data.frame(x=x,w=w) %>% group_by(x) %>% summarise(n=n())
expect_equal(nrow(res), 11L)
})

test_that("data.table invalid .selfref issue (#475)", {
dt <- data.table(x=1:5, y=6:10)
expect_that((dt %>% group_by(x))[, z := 2L], not(gives_warning()))
dt <- data.table(x=1:5, y=6:10)
expect_that((dt %>% group_by(x) %>% summarise(z = y^2))[, foo := 1L], not(gives_warning()))
})

0 comments on commit 6d789f0

Please sign in to comment.