Skip to content
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

coalesce vectors of type list #3712

Open
mattdowle opened this issue Jul 18, 2019 · 2 comments
Open

coalesce vectors of type list #3712

mattdowle opened this issue Jul 18, 2019 · 2 comments

Comments

@mattdowle
Copy link
Member

@mattdowle mattdowle commented Jul 18, 2019

Thanks to Antoine Fabri for reporting here:
https://twitter.com/antoine_fabri/status/1151965060522237971
NB: there are 4 images in that tweet to consider.

@mattdowle mattdowle added the dev label Jul 18, 2019
@mattdowle mattdowle added this to the 1.12.4 milestone Jul 18, 2019
@jangorecki jangorecki self-assigned this Jul 29, 2019
@jangorecki
Copy link
Member

@jangorecki jangorecki commented Jul 29, 2019

differences reported in tweet:

coalesce(list(1, NA), list(NA, 2))
#  The first argument is a list, data.table or data.frame. In this case there should be no other arguments provided.

is a result of API improvement implemented in 1f0bcc7 where x and ... has been merged into single input argument.
We should decide if we want to keep that improvement or be consistent to hutils/dplyr, and go back to distinct input argument x and ....

coalesce(list(c(1, NA), c(NA, 2)))
#[1] 1 2

there is no gain for being consistent to dplyr/hutils in this case as they do nothing, this is related to point 1 above.

never heard about tidy dots, could anyone provide a reference to R manual? or is it tidyverse-only feature?
AFAIU we already handle same input that dplyr requires !!!, this is related to point 1 above, where we merge x and ....

coalesce(1:2, 1:3)
#Error in coalesce(1:2, 1:3) : 
  Item 2 is length 3 but the first item is length 2. Only singletons are recycled.

error, as it is now, it is definitely better, same as dplyr, unlike hutils

coalesce(factor(NA,"a"), factor("a",c("a","b")))
#Error in coalesce(factor(NA, "a"), factor("a", c("a", "b"))) : 
  Item 2 is a factor but its levels are not identical to the first item's levels.

being consistent to dplyr/hutils here doesn't seem to be great, they result into NAs. Ideally would be to merge factor levels. I would say current DT behaviour is acceptable, if we want to change this we should merge levels.

@jangorecki jangorecki removed their assignment Jul 29, 2019
@mattdowle mattdowle removed the dev label Sep 19, 2019
@mattdowle mattdowle removed this from the 1.12.4 milestone Sep 19, 2019
@mattdowle mattdowle added this to the 1.13.0 milestone Sep 19, 2019
@mattdowle mattdowle removed this from the 1.12.7 milestone Dec 8, 2019
@mattdowle mattdowle added this to the 1.12.9 milestone Dec 8, 2019
@jangorecki
Copy link
Member

@jangorecki jangorecki commented Jun 17, 2020

related change coming to dplyr tidyverse/dplyr#5334

@mattdowle mattdowle removed this from the 1.13.1 milestone Oct 17, 2020
@mattdowle mattdowle added this to the 1.13.3 milestone Oct 17, 2020
@mattdowle mattdowle removed this from the 1.14.1 milestone Aug 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants