-
Notifications
You must be signed in to change notification settings - Fork 26
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
NA-row Handling on timeseries functions #45
Comments
You mean the user has the expectation they will not have any NA's returned to them if they use na.rm? I think it should still return NA if an entire row is NA; a warning might be helpful to the user or modify the documentation to indicate that the na.rm option makes the tool calculate given physical parameter based on non-NA data, if all data is NA, then it returns an NA. If it removes a row, then the data.frame dimensions get changed - I find that annoying when that happens. |
Interesting sum(c(NA,NA,NA), na.rm=TRUE)
[1] 0 |
max(c(NA,NA,NA), na.rm=TRUE)
[1] -Inf
Warning message:
In max(c(NA, NA, NA), na.rm = TRUE) :
no non-missing arguments to max; returning -Inf |
Huh. This is almost getting philosophical... the sum of nothing is 0 but the max of nothing is -Inf. I like the way max() returns a warning message |
Sometimes when defining APIs, it gets philosophical. I just wanted to know what base functions were doing with this challenge. Clearly, it's kinda weird behavior that I don't think we'd want to adopt (return -Inf or 0, nope). Let's call I propose applying this style of handling to all other timeseries functions. |
That sounds good to me. On Thu, Dec 4, 2014 at 5:02 PM, Luke Winslow [email protected]
Jacob A. Zwart |
We have "na.rm" option, but what happens if an entire row is NA? (#43) If we skip such a row, we will still be returning an NA, breaking the API contract.
Should we drop that row? Should we return an NA? Should we throw an error?
The text was updated successfully, but these errors were encountered: