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
make.index.unique() can create unsorted index #241
Comments
The functional change in this commit is adding eps when this is newindex_real[i] <= newindex_real[i-1] instead of when this is true: index_real[i-1] == index_real[i] This ensures the new index is always in increasing order, even observation following a contiguous block of duplicate timestamps less than the sum of the epsilons. The memcpy ensures 'newindex' is equal to the current 'index' the loop, which also means we longer need 'index_real'. Fixes #241.
Main consideration here is if |
I would like nanosecond resolution in xts, but that will take a bit of work. This problem could theoretically exist even with higher resolution index timestamps, but it would be less probable. The current solution in this branch works around the issue by always checking and ensuring that the value of |
This sounds correct to me. It has always been possible that make.index.unique could push things past the next observed index, and that problem only got worse as reported data moved beyond millisecond precision. Until xts supports nano-scale indexes, checking and warning the user that they may be doing something unintended with make.index.unique seems the best solution. |
I showed a possible solution to this problem (#190 (comment)) and yes, it is a lot of work, but in my opinion it is worth taking up this effort. The key is to not break xts R API for external packages. If you decide on the proposed solution, then of course I will be very committed to help as much as possible (especially in the case of C code). |
This can happen if the cumulative epsilon for a set of duplicate index values is larger than the first unique index value that follows. We will overwrite that non-duplicate index value with the prior index value + eps when this happens, and warn the user. See #241.
make.index.unique()
should add a smalleps
to duplicate index values. This often works, but can fail when there are consecutive observations with the same timestamp, and first observation after the block of duplicate timestamps is less than the cumulative eps.It would be nice if this could be fixed, but floating point rounding error is likely to be a problem in the cases this may occur. A warning should be raised, at minimum.
Session Info
The text was updated successfully, but these errors were encountered: