Fix underlying cause of bad ungroup() behavior through as_tibble()

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)

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
 

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)
 }
 

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)
 }
 
 

tests/testthat.R

@@ -1,5 +1,4 @@
 library(testthat)
 library(tibbletime)
-library(dplyr)
 
 test_check("tibbletime")

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)
+})

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
+  )
+})
+
+
+