-
-
Notifications
You must be signed in to change notification settings - Fork 71
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
[.xts returns all data when i is not a valid date/time #96
Comments
This makes me lean toward having the window function that I was proposing, have a base function that returns indexes. Essentially, subsetting with window should behave the same as subsetting with []. The idea would be to have subset process the range strings (as applicable) and call window_return_indexes. One wrinkle here is that the window.zoo function says that if start = NULL, this is interpreted as no constraint on the start date. (And similarly with end). |
I should clarify here. The point is not that window is automatically smarter (though it might help to have an NA check). The big point is there are some ambiguous cases - like what happens with an NA date - where reasonable people might disagree on the answer. Anyway - whatever standard is picked in subset for a range, I should make sure I follow it - and it may not be obvious. This says to me that I should make the effort to create a base window method that can be reused by subset to ensure consistency. So it means more work for me but is probably the right thing to do. |
The prior commit did not address the case when the string used to subset an xts object is only the range separator (i.e. "/" or "::"). Also add tests to ensure basic range-based subsets work, and that an empty string also returns the entire data set. See #96.
This will cause a failure in 'R CMD check' in R-3.6.0. This was introduced in 16bed0f as part of #96 (prevent subset from returning all data when 'i' is not a valid date- time). This should only be an issue when 'intervals' is length-1 and the first 3 logical comparisons are TRUE. So we only need to check whether the first element of 'intervals' is an empty string. Also add unit tests for basic functionality, and add some fuzz tests too. Fixes #280.
I thought the issue occured in/around lines 107-126 of
.parseISO8601
. At line 115, boths
ande
areNA
, which causes them to be set to the min/max of the index values, respectively, whenstart
andend
aren't missing. My guess is that we could throw an error thatx
(the character date) isn't in the range if boths
ande
areNA
...But Jeff suggested that the fix should stay out of
.parseISO8601
. And that the issue is howNA
is being handled in the subset itself. xts diverges from base R, in that we return nothing instead of all NAs, if usingi = NA
in the subset call. His take is that this case (either from or to areNA
) should behave like subsetting with a date outside of the set (i.e. empty).Thanks to Garrett See for the report.
The text was updated successfully, but these errors were encountered: