diff --git a/NAMESPACE b/NAMESPACE index 6727ae9..a3decfe 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -8,7 +8,6 @@ S3method(as_period,default) S3method(as_period,tbl_time) S3method(as_tbl_time,default) S3method(as_tbl_time,tbl_df) -S3method(as_tibble,grouped_tbl_time) S3method(as_tibble,tbl_time) S3method(ceiling_index,default) S3method(ceiling_index,hms) diff --git a/NEWS.md b/NEWS.md index 5ab8950..ff5900e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,11 @@ # tibbletime (development version) -* [Fix Ungroup Issue](https://github.com/business-science/tibbletime/issues/91) +* Coercing a grouped tbl_time object to tibble with `as_tibble()` now drops + groups and returns a bare tibble. The previous behavior of returning a + grouped tibble was incorrect and let to faulty behavior in other functions. + +* Fixed an issue related to `dplyr::ungroup()` in dplyr 1.0.0 where + ungrouping would not return an ungrouped tbl_time (#91). # tibbletime 0.1.4 diff --git a/R/coercion.R b/R/coercion.R index 292d45c..d957dfc 100644 --- a/R/coercion.R +++ b/R/coercion.R @@ -68,24 +68,13 @@ as_tbl_time.tbl_df <- function(x, index = NULL, ...) { #' @export #' @importFrom tibble as_tibble as_tibble.tbl_time <- function(x, ...) { - - # Remove index_* attributes - for(attrib in index_attributes()) { - attr(x, attrib) <- NULL - } - - tibble::new_tibble(x, ..., nrow = nrow(x)) + new_bare_tibble(x) } -#' @export -#' @importFrom tibble as_tibble -as_tibble.grouped_tbl_time <- function(x, ...) { - - # Remove index_* attributes - for(attrib in index_attributes()) { - attr(x, attrib) <- NULL - } - - tibble::new_tibble(x, ..., nrow = nrow(x), class = "grouped_df") +# new_tibble() currently doesn't strip attributes +# https://github.com/tidyverse/tibble/pull/769 +new_bare_tibble <- function(x, ..., class = character()) { + x <- vctrs::new_data_frame(x) + tibble::new_tibble(x, nrow = nrow(x), ..., class = class) } diff --git a/R/compat-dplyr.R b/R/compat-dplyr.R index 6e4d495..e4f2d7c 100644 --- a/R/compat-dplyr.R +++ b/R/compat-dplyr.R @@ -136,16 +136,10 @@ group_by.tbl_time <- function(.data, ...) { #' @export #' @importFrom dplyr ungroup -#' ungroup.tbl_time <- function(x, ...) { #reconstruct(NextMethod(), x) - # copy_.data <- new_tbl_time(x, get_index_quo(x), get_index_time_zone(x)) - # ret <- reconstruct(NextMethod(), copy_.data) - idx_quo <- get_index_quo(x) - idx_tz <- get_index_time_zone(x) - tbl <- dplyr::ungroup(tibble::as_tibble(x)) - new_tbl_time(tbl, idx_quo, idx_tz) - + copy_.data <- new_tbl_time(x, get_index_quo(x), get_index_time_zone(x)) + reconstruct(NextMethod(), copy_.data) } diff --git a/tests/testthat.R b/tests/testthat.R index 47230eb..584261f 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -1,5 +1,4 @@ library(testthat) library(tibbletime) -library(dplyr) test_check("tibbletime") diff --git a/tests/testthat/test-coercion.R b/tests/testthat/test-coercion.R new file mode 100644 index 0000000..bf4f675 --- /dev/null +++ b/tests/testthat/test-coercion.R @@ -0,0 +1,18 @@ +test_that("coercing tbl_time to tibble works", { + df <- as_tbl_time(FANG, date) + x <- as_tibble(df) + + expect_s3_class(x, c("tbl_df", "tbl", "data.frame"), exact = TRUE) + + # Ensure attributes are dropped + expect_null(attr(x, "index_quo")) + expect_null(attr(x, "index_time_zone")) +}) + +test_that("coercing grouped_tbl_time to tibble drops groupedness", { + df <- as_tbl_time(FANG, date) + gdf <- group_by(df, symbol) + x <- as_tibble(gdf) + + expect_s3_class(x, c("tbl_df", "tbl", "data.frame"), exact = TRUE) +}) diff --git a/tests/testthat/test_compat-dplyr.R b/tests/testthat/test_compat-dplyr.R new file mode 100644 index 0000000..f89e636 --- /dev/null +++ b/tests/testthat/test_compat-dplyr.R @@ -0,0 +1,18 @@ +test_that("ungroup() works", { + df <- tibble::tibble( + group = c("g1", "g1", "g2"), + date = as.Date(c("2017-12-01", "2017-12-02", "2017-12-03")) + ) + + df <- as_tbl_time(df, date) + df <- dplyr::group_by(df, group) + + expect_s3_class( + dplyr::ungroup(df), + c("tbl_time", "tbl_df", "tbl", "data.frame"), + exact = TRUE + ) +}) + + + diff --git a/tests/testthat/test_grouped_df.R b/tests/testthat/test_grouped_df.R deleted file mode 100644 index 62196d1..0000000 --- a/tests/testthat/test_grouped_df.R +++ /dev/null @@ -1,31 +0,0 @@ -context("grouped_df testing") - -# Test objects - -test_time <- tibble::tibble( - group = c("g1", "g1", "g2"), - date = c(as.Date("2017-12-01"), as.Date("2017-12-02"), as.Date("2017-12-03")), - value = c(1, 2, 3), - value2 = c(2, 5, 6), - value3 = c(1, 3, 7) -) - -test_grouped_tbl_time <- as_tbl_time(test_time, date) %>% - group_by(group) - - - -# Tests - -test_that("Ungroup works", { - test_ungrouped_tbl_time <- test_grouped_tbl_time %>% - ungroup() - - test_ungrouped_tbl_time %>% class() - - expect_true(!"grouped_tbl_time" %in% class(test_ungrouped_tbl_time)) - -}) - - -