-
Notifications
You must be signed in to change notification settings - Fork 990
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
Should rbindlist(..., fill=TRUE) return NA_logical_ in list columns? #4198
Comments
PR with fix provided if we want to make the change |
Current behaviour seems fine to me.
IMO it should not be NA because:
|
I'm leaning towards Jan's point. Current behavior of empty element is actually a list's way of representing missing (there isn't any object to point to). We could construct an example where each item of the list was a logical vector, each item being the result of some computation. In such a case, 3 different states might need to be represented: length 0 logical, length 1 NA logical, and missing computation. If length 1 NA logical was used for missing, those 2 couldn't be distinguished. Would changing the print method suffice? Instead of nothing being printed, how about |
also agree w Jan, in particular about length 0 --> length 1. I'm using lengths(x)>0 a lot to filter rows by empty list columns. we could I guess put logical() there instead, is there any advantage of logical() over NULL though? |
What if we just instead add a sentence to the documentation noting the behaviour in the case of a missing list column: Current entry for the fill argument:
Proposed:
|
Doc change looks good. Plus the print method change I suggested too? |
From #4196:
When filling a list column, rbindlist departs from the behaviour of all other column types, and returns NULL elements instead of NA:
Expected:
Should we change this behaviour for list columns to fill the rows with NA values to match the behaviour of fill=TRUE for other column types?
The text was updated successfully, but these errors were encountered: