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
220 stars 71 forks source link

S3 method conflict with zoo #287

Closed joshuaulrich closed 5 years ago

joshuaulrich commented 5 years ago

Kurt Hornik noticed, and Achim Zeileis reported that zoo's method for as.zoo.xts is overwritten by xts. So zoo's S3 registry entries are overwritten when xts is loaded.

It looks like the xts method only exists because it didn't exist in zoo at the time. I think the package that owns the class should define the conversion method, so the as.zoo* methods should be in zoo.

Achim noted the difference in as.zoo.xts() between zoo and xts is small (with xts being better). He thinks we should just move xts' version into zoo, and I agree.

joshuaulrich commented 5 years ago

I tried to address this and got a couple test failures when I removed xts' method without updating zoo's. The code below illustrates the difference:

R> x <- xts::.xts(, 1)
R> xts:::as.zoo.xts(x)
Data:
numeric(0)

Index:
[1] "1969-12-31 18:00:01 CST"
R> zoo:::as.zoo.xts(x)
1969-12-31 18:00:01
                 NA

So it seems like we would need a zoo release prior to removing it from xts and updating the zoo dependency version. Or maybe there's a way to only register it in xts if zoo is less than a specific version.

Achim suggested try using an .onLoad() hook similar to that in zoo/R/zzz.R which conditionally registers ggplot2 methods.

linusjf commented 4 years ago

So has this been fixed? I'm getting the same message on R for Termux, version 3.6.2.

joshuaulrich commented 4 years ago

Yes, this is fixed. I'm working with CRAN to get the patched version released.

linusjf commented 4 years ago

Ok, thanks. I've suppressed the messages using suppressMessages.