Skip to content

Commit f98ebc9

Browse files
committedJun 10, 2020
dplyr issue addressed I think.
cc @hlapp
1 parent afbaeaf commit f98ebc9

File tree

1 file changed

+6
-2
lines changed

1 file changed

+6
-2
lines changed
 

‎tests/testthat/test_01_utils.R

+6-2
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,12 @@ test_that("coalesce_() works correctly", {
126126
# (1) dplyr::coalesce is strict about type, and for some reason vectors
127127
# that are all NAs default to type logical
128128
testthat::expect_is(dta[, "col1"], "logical")
129-
testthat::expect_error(dplyr::coalesce(dta$col1, dta$col3, last))
130-
testthat::expect_error(dplyr::coalesce(dta$col3, last)) # col3 is a factor
129+
130+
# EDIT: I believe these ungraceful behaviors in dplyr are resolved
131+
# in the most recent version, and so these no longer throw errors:
132+
133+
## testthat::expect_error(dplyr::coalesce(dta$col1, dta$col3, last))
134+
## testthat::expect_error(dplyr::coalesce(dta$col3, last)) # col3 is a factor
131135
# (2) dplyr::coalesce doesn't gracefully deal with NULL arguments, which
132136
# can result from referencing column names that aren't there, requiring
133137
# a conditional testing for presence of the column

2 commit comments

Comments
 (2)

hlapp commented on Jun 10, 2020

@hlapp
Contributor

That's the bandaid fix, yes. But these tests are there for a reason, namely to check that the ungraceful behavior of dplyr::coalesce remains present as the raison d'être for coalesce()_.

I've been away for a week, so haven't had time to investigate yet. The issue should not be closed until we have investigated and decided that coalesce_() remains needed.

cboettig commented on Jun 10, 2020

@cboettig
MemberAuthor
Please sign in to comment.