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

Move index attributes to index object #245

Closed joshuaulrich closed 5 years ago

joshuaulrich commented 6 years ago

The xts class was originally implemented with the index class and index timezone attributes attached to the xts object itself. That means the xts class needs to know about all possible index classes, which makes it difficult to extend xts to use a new index class.

Jeff started to move the index-specific attributes from the xts object to the index object. See the commit message for e4edc53fdde423513b0a730d5d8c7fb7ddeefe81.

o added tzone and tclass functions as aliases to indexTZ and indexClass, respectively. Eventually will Deprecate/Defunct the former.

Most places currently set attributes on both objects, because it's not clear which components of xts depend on the index-specific attributes being attached to either object (xts or index).

We would first need to ensure that all attempts to access the index class or timezone are getting and returning values from the index object (note that this applies to R and C code). Then we would need to remove all attempts to set attributes on the xts object... and see what breaks.

joshuaulrich commented 6 years ago

This is required before #190 can be completed.

joshuaulrich commented 5 years ago

Reverse-dependency checks indicate issues in the following packages: geoSpectral, PRISM.forecast, and RcppXts.

geoSpectral

At least one example fails:

* checking examples ... ERROR
Running examples in 'geoSpectral-Ex.R' failed
The error most likely occurred in:

> ### Name: SpcHeaderAdd
> ### Title: Set a field of the @header slot of a 'SpcHeader' class object
> ### Aliases: SpcHeaderAdd SpcHeaderAdd,SpcHeader-method
>
> ### ** Examples
>
> sp=spc.example_spectra()
Error in validityMethod(as(object, superClass)) :
  tz1.set == tz2.set is not TRUE
Calls: spc.example_spectra ... anyStrings -> isTRUE -> validityMethod -> stopifnot
Execution halted

And at least one test fails for what looks like a similar reason:

> library(testthat)
> library(geoSpectral)
>
> test_check("geoSpectral")
── 1. Error: (unknown) (@test_Spectra_Constructor.R#4)  ────────────────────────
tz1.set == tz2.set is not TRUE
1: spc.example_spectra() at testthat/test_Spectra_Constructor.R:4
2: Spectra(abs, Wavelengths = lbd, Units = Units, ShortName = "a_nap", LongName = "Absorption coefficient by non-algal particles")
3: new("Spectra", out, Spectra = Spectra, Wavelengths = Wavelengths, Units =    Units,
       header = header, ...)
4: initialize(value, ...)
5: initialize(value, ...)
6: validObject(.Object)
7: anyStrings(validityMethod(as(object, superClass)))
8: isTRUE(x)
9: validityMethod(as(object, superClass))
10: stopifnot(tz1.set == tz2.set)

══ testthat results  ═══════════════════════════════════════════════════════════
OK: 0 SKIPPED: 0 FAILED: 1
1. Error: (unknown) (@test_Spectra_Constructor.R#4)

Error: testthat unit tests failed
Execution halted

PRISM.forecast

Example fails:

* checking examples ... ERROR
Running examples in 'PRISM.forecast-Ex.R' failed
The error most likely occurred in:

> ### Name: prism
> ### Title: PRISM function
> ### Aliases: prism
> 
> ### ** Examples
> 
> prism_data = load_5y_search_data('0610')
> data = prism_data$claim.data[1:200] # load claim data from 2006-01-07 to 2009-10-31
> data[200] = NA # delete the data for the latest date and try to nowcast it.
> 
> data.early = prism_data$claim.earlyData # load claim prior to 2006
> GTdata = prism_data$allSearch[1:200] # load Google trend data from 2006-01-07 to 2009-10-31
>
> result = prism(data, data.early, GTdata) # call prism method
Warning in tclass.xts(x) : index does not have a ‘tclass’ attribute
Warning in tclass.xts(x) :
  object does not have a ‘tclass’ or ‘.indexCLASS’ attribute
Error in class(xx) <- cl : attempt to set an attribute on NULL
Calls: prism ... as.matrix -> as.matrix.xts -> index -> index.xts -> .POSIXct
Execution halted

RcppXts

Test fails:

> library(RcppXts)
> X  <- xts(1:4, order.by=Sys.time()+0:3)
> # ...
> xtsRbind(X, X, TRUE)
Error in index.xts(x) : unsupported 'tclass' indexing type:
Calls: <Anonymous> ... coredata -> coredata.xts -> format -> index -> index.xts
In addition: Warning messages:
1: In tclass.xts(x) : index does not have a 'tclass' attribute
2: In tclass.xts(x) :
  object does not have a 'tclass' or '.indexCLASS' attribute
3: In tzone.xts(x) : index does not have a 'tzone' attribute
4: In tzone.xts(x) :
  object does not have a 'tzone' or '.indexTZ' attribute
5: In tzone.xts(x) : index does not have a 'tzone' attribute
6: In tzone.xts(x) :
  object does not have a 'tzone' or '.indexTZ' attribute
7: In tclass.xts(x) : index does not have a 'tclass' attribute
8: In tclass.xts(x) :
  object does not have a 'tclass' or '.indexCLASS' attribute
Execution halted
joshuaulrich commented 5 years ago

The RcppXts bug is in the xts C function do_rbind_xts:

X  <- xts(1:4, order.by=Sys.time()+0:3)
rbx <- function(..., dup = FALSE, deparse.level = 1) {
  .External("rbindXts", dup = dup, ..., PACKAGE = "xts")
}
rbx(X, X, dup = TRUE)

The PRISM.forecast bug can be replicated with:

data(sample_matrix)
x <- as.xts(sample_matrix)
drop(x[,1])-coredata(x[,1:2])

The geoSpectral bug requires debugging S4, which I'm not particularly good at.

joshuaulrich commented 5 years ago

It looks like the PRISM.forecast error is due to malformed xts objects being created in the Ops.xts() method. I will create a new issue to document those. That issue will need to be closed before this can be merged.