Skip to content

Commit

Permalink
combine() returns logical() (closes tidyverse#3365)
Browse files Browse the repository at this point in the history
  • Loading branch information
zeehio committed Mar 1, 2018
1 parent 27becad commit b640a7a
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 8 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Expand Up @@ -69,6 +69,8 @@

* Added an `.onDetach()` hook that allows for plyr to be loaded and attached without the warning message that says functions in dplyr will be masked, since dplyr is no longer attached (#3359, @jwnorman).

* `combine()` returns `logical()` when all inputs are `NULL` (or when there are no inputs) (#3365, @zeehio).

# dplyr 0.7.4

* Fix recent Fedora and ASAN check errors (#3098).
Expand Down
4 changes: 2 additions & 2 deletions src/bind.cpp
Expand Up @@ -399,7 +399,7 @@ List cbind_all(List dots) {
// [[Rcpp::export]]
SEXP combine_all(List data) {
int nv = data.size();
if (nv == 0) stop("combine_all needs at least one vector");
if (nv == 0) return LogicalVector();

// get the size of the output
int n = 0;
Expand All @@ -412,7 +412,7 @@ SEXP combine_all(List data) {
for (; i < nv; i++) {
if (!Rf_isNull(data[i])) break;
}
if (i == nv) stop("no data to combine, all elements are NULL");
if (i == nv) return LogicalVector();

// collect
boost::scoped_ptr<Collecter> coll(collecter(data[i], n));
Expand Down
8 changes: 2 additions & 6 deletions tests/testthat/test-combine.R
@@ -1,14 +1,10 @@
context("combine")

test_that("combine handles NULL (1596)", {
test_that("combine handles NULL (#1596, #3365)", {
expect_equal(combine(list(NULL, 1, 2)), c(1, 2))
expect_equal(combine(list(1, NULL, 2)), c(1, 2))
expect_equal(combine(list(1, 2, NULL)), c(1, 2))
expect_error(
combine(list(NULL, NULL)),
"no data to combine, all elements are NULL",
fixed = TRUE
)
expect_equal(combine(list(NULL)), logical())
})

test_that("combine complains about incompatibilites", {
Expand Down

0 comments on commit b640a7a

Please sign in to comment.