IQSS / Amelia

Amelia: A Package for Missing Data
http://gking.harvard.edu/amelia
61 stars 17 forks source link

fix bugs with [ code assuming that drop = TRUE #6

Closed jrnold closed 7 years ago

jrnold commented 8 years ago

This fixes some, but not all of the cases, in which [ is used to extract a single column from a data frame as a vector, and assumes that the value of the drop argument is TRUE. I got this bug when using a data_frame class from dplyr, because the [.data_frame method always returns a data_frame and does not drop dimensions. I fixed a few of the instances by replacing some problematic cases of [,i] with [[i]], but the idiom of extracting a single column with code similar to [,i] seems to be used throughout the code base.

mattblackwell commented 8 years ago

Thanks for catching this. This is tricky because the "x" object can currently either be a matrix or a data.frame. We originally wrote the code to respect the choices so that the output matched the input. Obviously this was before stuff like data_frame. The issue is that x[[i]] won't work on vanilla matrix objects. So we'll have to give this some thought and possibly convert everything to a data.frame and use either x[[i]] or x[,i].

jrnold commented 8 years ago

That's annoying. R implicitly dropping dimensions was an original sin of the language. This seemingly redundant piece of code at the start could work,

if (is.data.frame(x)) x <- as.data.frame(x)
jrnold commented 7 years ago

I had a student run into this error again, so I opened up a new PR #7, which is just the hack that I proposed in my last message.