Documentation needs updating
mlamias opened this issue · 5 comments
I believe the documentation needs to be updated. Currently the doc for the na_ functions reads:
"Both options have the issue, that NAs at the beginning (or for nocb at the end) of the time series cannot be imputed (since there is no last value to be carried forward present yet). In this case there are remaining NAs in the imputed time series. Since this only concerns very few values at the beginning of the series, na_remaining offers some quick solutions to get a series without NAs back."
This no longer appears to be true. It seems NA values at the begging or end of a time series are now replaced.
I would personally love to see an option to replace or not replace NAs at the beginning or end of a time series (i.e. only replace NAs once non-NA values commence).
Was this handled differently under a previous version that I can still access?
Hello @mlamias ,
your citation is from documentation file of na_locf.
You are right ... indeed this can be easily misunderstood.
Currently it sounds like keeping the remaining NAs after last observation carried forward is the default.
(it is not)
I'll fix this :-)
About your request for an option to keep the NAs at the beginning, there is the na_remaining parameter.
na_locf(x, na_remaining = "keep")
will keep the NAs at the beginning/end.
na_locf(x, na_remaining = "rev")
performs nocb / locf from the reverse direction to impute the remaining NAs. That is the default, if you just run na_locf without further specifying the na_remaining parameter.
And then there is
na_locf(x, na_remaining = "mean")
which replaces the remaining NAs from na_locf with the mean
and also
na_locf(x, na_remaining = "rm")
which removes the NA observations at the beginning completely from the time series / shortens the time series.
To sum up:
- "keep" - to return the series with NAs
- "rm" - to remove remaining NAs
- "mean" - to replace remaining NAs by overall mean
- "rev" - to perform nocb / locf from the reverse direction (DEFAULT)
Like you said, the documentation sounds somehow like "keep" is the default, while in reality "rev" is the default.
Good hint, will be updated in the next version.
Thanks for your help :)
Excellent! Thanks for the information and the fix. This is excellent. I love this package by the way! It's been critical for my work on COVID and Influenza!
Fix is on Github. The na_locf description is more extensive now and behavior of the na_remaining parameter should be more clear. ( https://github.com/SteffenMoritz/imputeTS/blob/master/man/na_locf.Rd )
Will also be on CRAN with the next CRAN update of imputeTS.
I also added a small shoutout in the package news to thank you for reporting this:
https://github.com/SteffenMoritz/imputeTS/blob/master/NEWS.md
Always nice to have some help improving the package : )
Fix is on Github.
That is fantastic! Thank you so much for taking my suggestion and making an improvement! I really appreciate it.
I also added a small shoutout in the package news to thank you for reporting this:
Very kind of you!
Always nice to have some help improving the package : )
Absolutely. Would you be open to contributions from a statistician like me? I have a number of ideas and methods that might be helpful in this package? I'm not sure if you'd be open to any collaboration or not. If so, please let me know. Thank you again!
Oh would have answered earlier - I didn't get a notification for your reply. Maybe, because the reply was after I closed the issue. Anyway, happy new year!
Yes, I am absoluteIy open to contributions! There are a lot of things that still can be added or improved upon. I also add all contributor as contributors/authors on CRAN. Just drop me a mail (steffen.moritz10@gmail.com) about what you have in mind. Certainly beneficial to shortly discuss changes beforehand. I think one of our goals should be to keep the core functionality as easy to understand and easy to use as possible (while still enabling advanced features for users that want to dig deeper into the topic/package).