Skip to content
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export(type_sum)
export(validate_tibble)
export(view)
exportClasses(tbl_df)
if (getRversion() >= "4.0.0") S3method(cbind, tbl_df)
if (getRversion() >= "4.0.0") S3method(rbind, tbl_df)
import(ellipsis)
import(lifecycle)
import(rlang)
Expand All @@ -96,6 +98,7 @@ importFrom(vctrs,vec_as_names_legacy)
importFrom(vctrs,vec_as_subscript2)
importFrom(vctrs,vec_assign)
importFrom(vctrs,vec_c)
importFrom(vctrs,vec_cbind)
importFrom(vctrs,vec_is)
importFrom(vctrs,vec_names2)
importFrom(vctrs,vec_ptype_abbr)
Expand Down
13 changes: 13 additions & 0 deletions R/class-tbl_df.R
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,19 @@ cnd_names_non_na <- function(name) {
}
}

#' @rawNamespace if (getRversion() >= "4.0.0") S3method(rbind, tbl_df)
if (getRversion() >= "4.0.0") rbind.tbl_df <- function(...) {
# deparse.level is part of the interface of the generic but not passed along
# for data frame methods

vec_rbind(...)
}

#' @rawNamespace if (getRversion() >= "4.0.0") S3method(cbind, tbl_df)
if (getRversion() >= "4.0.0") cbind.tbl_df <- function(...) {
vec_cbind(...)
Copy link
Member

@DavisVaughan DavisVaughan Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it is reasonable that if users want to use the optional arguments in vec_c/rbind(), then they should just use vec_c/rbind() instead, but I wanted to check here if that is also what you are thinking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Should we do vec_cbind(!!!list(...)) instead, to avoid passing through optional arguments?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would work yes. Or should that be list2()? Or would it be weird to use !!! with base generics? I don't really know what to think about these mixed interfaces.

}

# Errors ------------------------------------------------------------------

error_names_must_be_non_null <- function() {
Expand Down
1 change: 1 addition & 0 deletions R/tibble-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#' @import ellipsis
#' @importFrom vctrs vec_as_location vec_as_location2 vec_as_names vec_as_names_legacy vec_c
#' @importFrom vctrs vec_is vec_rbind vec_recycle vec_size vec_slice vec_assign
#' @importFrom vctrs vec_cbind
#' @importFrom vctrs unspecified vec_as_subscript2 num_as_location vec_ptype_abbr
#' @importFrom vctrs vec_names2 vec_set_names
#' @aliases NULL tibble-package
Expand Down
16 changes: 16 additions & 0 deletions tests/testthat/test-class-tbl_df.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ test_that("names<-()", {
)
})

test_that("rbind()", {
skip_if_not(getRversion() >= "4.0.0")
expect_equal(rbind(tibble(a = 1)), tibble(a = 1))
expect_equal(rbind(tibble(a = 1), data.frame(a = 2)), tibble(a = 1:2))
expect_equal(rbind(tibble(a = 1), data.frame(b = 2)), tibble(a = c(1, NA), b = c(NA, 2)))
expect_equal(rbind(data.frame(a = 1), tibble(a = 2)), data.frame(a = 1:2))
})

test_that("cbind()", {
skip_if_not(getRversion() >= "4.0.0")
expect_equal(cbind(tibble(a = 1)), tibble(a = 1))
expect_equal(cbind(tibble(a = 1), data.frame(b = 2)), tibble(a = 1, b = 2))
expect_equal(cbind(tibble(a = 1), data.frame(b = 2:3)), tibble(a = c(1, 1), b = 2:3))
expect_equal(cbind(data.frame(a = 1), tibble(b = 2)), data.frame(a = 1, b = 2))
})

test_that("output test", {
skip_if_not_installed("testthat", "3.0.0.9000")
skip_if_not_installed("lifecycle", "0.2.0.9000")
Expand Down