search

amices / mice

Multivariate Imputation by Chained Equations
https://amices.org/mice/
GNU General Public License v2.0
433 stars 107 forks source link

Function 'ampute' in package 'mice' #488

Closed kmc8c3 closed 1 year ago

kmc8c3 commented 2 years ago

Hello,

I believe there is a minor coding issue on line 49 of the function code 'ampute'.

The line of code is supposed to match the length of frequencies/frequency vector to the number of patterns specified when the length of frequency vector is greater than the number of patterns.

Currently, it throws an error: Error in sample.int(n = nrow(patterns.new), size = nrow(data), replace = TRUE, : incorrect number of probabilities In addition: Warning message: Length of vector with relative frequencies does not match #patterns and is therefore changed to 0.33

So, this line

freq <- freq[seq_along(nrow(patterns))]

needs to be

freq <- freq[seq_along(1:nrow(patterns))]

Hope it makes sense.

AndrewLawrence commented 2 years ago

Good spot!

For programming use, the colon operator 1:x is a bit unsafe as if x ever holds the value zero you would iterate through 1, 0 without (always) throwing an error. There are similar problems if x is ever negative.

There's a base function that avoids this: seq_len, it first tries to coerce input to a non-negative integer and will immediately throw an error if this doesn't work..

So:

freq <- freq[seq_len(nrow(patterns))]

would probably be a safer correction

stefvanbuuren commented 1 year ago

Line 249. This appears to be already solved in mice 3.14.12, so closing now.