New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

combine fails to combine a list of characters with NA [with pull request: #2209] #2203

Closed
zeehio opened this Issue Oct 26, 2016 · 0 comments

Comments

Projects
None yet
1 participant
@zeehio
Copy link
Contributor

zeehio commented Oct 26, 2016

Hi,

Thanks to everyone working on tibble, dplyr, tidyr... In general the tidyverse packages make a lot of people very happy to use R.

I have found a rather simple case where dplyr::combine fails, and the do.call rbind equivalent works as expected. I have tested this on the latest released dplyr and the current master (b205d1f) versions, both showing the same error. I found this error trying to unnest a data frame using tidyr, but my C++ skills and dplyr knowledge are not good enough to fix it.

library(dplyr)
do.call(rbind,list("a", "b", "c", NA, "e")) # Works

combine(list("a", "b", "c", NA, "e")) # Does not work
#Error in eval(substitute(expr), envir, enclos) : 
#  Can not automatically convert from character to logical. 

combine(list("a", "b", "c", NA_character_, "e")) # Workaround

This issue affects unnest in tidyr:

library(tibble)
library(tidyr)

aa <- tibble(ID = list("a", "b", "c", NA, "e"))
bb <- unnest_(aa, "ID") # I would expect bb == tibble(ID = c("a", "b", "c", NA, "e"))

Any help will be appreciated

zeehio added a commit to zeehio/dplyr that referenced this issue Oct 27, 2016

Add test case for tidyverse#2203
Combine should work with NA and characters

zeehio added a commit to zeehio/dplyr that referenced this issue Oct 27, 2016

combine works with NA and character (Closes tidyverse#2203)
This commit fixes tidyverse#2203 by adding compatibility to the character Collecter  so it accepts logical missing values and replaces them with NA_character_ as expected.

zeehio added a commit to zeehio/dplyr that referenced this issue Oct 27, 2016

@zeehio zeehio changed the title combine fails to combine a list of characters with NA combine fails to combine a list of characters with NA [with pull request: #2209] Oct 27, 2016

zeehio added a commit to zeehio/dplyr that referenced this issue Nov 7, 2016

Test cases: combine with NA (tidyverse#2203)
This commit tests if dplyr::combine accepts NA in all atomic classes:

 - logical
 - integer
 - factor
 - real
 - character
 - POSIXct
 - Date
 - complex

zeehio added a commit to zeehio/dplyr that referenced this issue Nov 7, 2016

Collecters compatible with NA values (Closes tidyverse#2203)
This commit fixes tidyverse#2203 by allowing the collecters to accept logical missing values, replacing
them with the specific NA value for each Rtype as expected.

zeehio added a commit to zeehio/dplyr that referenced this issue Nov 7, 2016

zeehio added a commit to zeehio/dplyr that referenced this issue Nov 13, 2016

zeehio added a commit to zeehio/dplyr that referenced this issue Nov 13, 2016

Fix issue tidyverse#2203. combine deals with NA, has stricter coercio…
…n rules

This commit fixes issue tidyverse#2203, allowing combine to deal with missing
values.

Additionally it restricts coercion rules, in particular coercing
logical to integer or double is not allowed anymore.

Other coercion cases will give warnings, if information may be
lost in the conversion, for instance when coercing integers with
classes, such as difftime.

zeehio added a commit to zeehio/dplyr that referenced this issue Nov 13, 2016

zeehio added a commit to zeehio/dplyr that referenced this issue Nov 13, 2016

Fix issue tidyverse#2203. combine deals with NA, has stricter coercio…
…n rules

This commit fixes issue tidyverse#2203, allowing combine to deal with missing
values.

Additionally it restricts coercion rules, in particular coercing
logical to integer or double is not allowed anymore.

Other coercion cases will give warnings, if information may be
lost in the conversion, for instance when coercing integers with
classes, such as difftime.

zeehio added a commit to zeehio/dplyr that referenced this issue Jan 27, 2017

zeehio added a commit to zeehio/dplyr that referenced this issue Jan 27, 2017

Fix issue tidyverse#2203. combine deals with NA, has stricter coercio…
…n rules

This commit fixes issue tidyverse#2203, allowing combine to deal with missing
values.

Additionally it restricts coercion rules, in particular coercing
logical to integer or double is not allowed anymore.

Other coercion cases will give warnings, if information may be
lost in the conversion, for instance when coercing integers with
classes, such as difftime.

zeehio added a commit to zeehio/dplyr that referenced this issue Jan 30, 2017

zeehio added a commit to zeehio/dplyr that referenced this issue Jan 30, 2017

Fix issue tidyverse#2203. combine deals with NA, has stricter coercio…
…n rules

This commit fixes issue tidyverse#2203, allowing combine to deal with missing
values.

Additionally it restricts coercion rules, in particular coercing
logical to integer or double is not allowed anymore.

Other coercion cases will give warnings, if information may be
lost in the conversion, for instance when coercing integers with
classes, such as difftime.

@krlmlr krlmlr closed this in 0833d5b Feb 1, 2017

@lock lock bot locked as resolved and limited conversation to collaborators Jun 8, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.