joshuaulrich / xts

Extensible time series class that provides uniform handling of many R time series classes by extending zoo.
http://joshuaulrich.github.io/xts/
GNU General Public License v2.0
219 stars 70 forks source link

merge.xts() should be commutative #191

Open joshuaulrich opened 7 years ago

joshuaulrich commented 7 years ago

The order that xts objects are passed to merge.xts can produce different results when the index classes differ. It seems reasonable to expect that the same result be returned regardless of the order the inputs are provided. This may, or may not, be related to #53.

For example:

library(xts)
Sys.setenv(TZ = "America/Chicago")
d <- c("1990-01-01", "1991-01-01")
x1 <- xts(1:2, as.Date(d))
x2 <- xts(3:4, as.POSIXct(d))
(y1 <- merge(x1, x2))
(y2 <- merge(x2, x1))

The results are:

R> (y1 <- merge(x1, x2))
           x1 x2
1990-01-01  1 NA
1990-01-01 NA  3
1991-01-01  2 NA
1991-01-01 NA  4
R> (y2 <- merge(x2, x1))
                    x2 x1
1989-12-31 18:00:00 NA  1
1990-01-01 00:00:00  3 NA
1990-12-31 18:00:00 NA  2
1991-01-01 00:00:00  4 NA
ggrothendieck commented 7 years ago

Perhaps you could just say that that is not supported. In zoo you get a warning. It gives you some answer but the result is not necessarily coirrect; however, at least you were warned about it.

joshuaulrich commented 7 years ago

@ggrothendieck Thanks for the comment!

I think we can return a sensible xts object if one index is Date and the other is POSIXct, since they're both time-based. We will need to be careful about timezones though. The problem is harder for zoo, since index types do not necessarily have a clear relationship.

The warning is an interesting idea. Not sure if it would be helpful or an annoyance though. I guess it depends on the final solution.