Move index attributes to index object #245

joshuaulrich · 2018-05-29T15:47:30Z

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 e4edc53.

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.

The text was updated successfully, but these errors were encountered:

joshuaulrich · 2018-05-29T16:14:14Z

This is required before #190 can be completed.

Mark `indexTZ()` and `indexTZ<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tzone" function instead. Replace calls to indexTZ() with calls to tzone(). Remove all uses of 'xts_IndexTZSymbol' in C code, including the macros 'GET_xtsIndexTZ' and 'SET_xtsIndexTZ'. Opportunistically remove .indexTZ and tzone attributes from objects created using prior versions of xts. Also remove "TZ" name attribute from `tzone<-.xts`. Rename R/indexTZ.R to R/tzone.R, and man/indexTZ.Rd to man/tzone.Rd. See #245.

Mark `indexClass()` and `indexClass<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tclass" function instead. Replace calls to indexClass() with calls to tclass(). Remove all uses of 'xts_IndexClassSymbol' in C code, including the macros 'GET_xtsIndexClass' and 'SET_xtsIndexClass'. Opportunistically remove .indexCLASS and tclass attributes from objects created using prior versions of xts. Rename R/indexClass.R to R/tclass.R and man/indexClass.Rd to man/tclass.Rd. See #245.

Mark `indexTZ()` and `indexTZ<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tzone" function instead. Replace calls to indexTZ() with calls to tzone(). Remove all uses of 'xts_IndexTZSymbol' in C code, including the macros 'GET_xtsIndexTZ' and 'SET_xtsIndexTZ'. Opportunistically remove .indexTZ and tzone attributes from objects created using prior versions of xts. Also remove "TZ" name attribute from `tzone<-.xts`. Rename R/indexTZ.R to R/tzone.R, and man/indexTZ.Rd to man/tzone.Rd. See #245.

Mark `indexClass()` and `indexClass<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tclass" function instead. Replace calls to indexClass() with calls to tclass(). Remove all uses of 'xts_IndexClassSymbol' in C code, including the macros 'GET_xtsIndexClass' and 'SET_xtsIndexClass'. Opportunistically remove .indexCLASS and tclass attributes from objects created using prior versions of xts. Rename R/indexClass.R to R/tclass.R and man/indexClass.Rd to man/tclass.Rd. See #245.

Mark `indexFormat()` and `indexFormat<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tformat" functions instead. Replace calls to indexFormat() with calls to tformat(). Remove all uses of 'xts_IndexFormatSymbol' in C code, including the macros 'GET_xtsIndexFormat' and 'SET_xtsIndexFormat'. Opportunistically remove .indexFORMAT attribute from objects created using prior versions of xts. Rename R/indexFormat.R to R/tformat.R. See #245.

Index attributes will no longer be attached to the xts object itself, so warn the user if they pass deprecated arguments to the xts constructor. See #245.

joshuaulrich · 2018-10-23T11:46:17Z

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 · 2018-10-23T12:54:43Z

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.

Calling lengthgets() creates a new object, which will not have the same attributes as the original object. Wait to copy the original index attributes to the new index until after lengthgets() is potentially called. See #245.

Mark `indexTZ()` and `indexTZ<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tzone" function instead. Replace calls to indexTZ() with calls to tzone(). Remove all uses of 'xts_IndexTZSymbol' in C code, including the macros 'GET_xtsIndexTZ' and 'SET_xtsIndexTZ'. Opportunistically remove .indexTZ and tzone attributes from objects created using prior versions of xts. Also remove "TZ" name attribute from `tzone<-.xts`. Rename R/indexTZ.R to R/tzone.R, and man/indexTZ.Rd to man/tzone.Rd. See #245.

Mark `indexClass()` and `indexClass<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tclass" function instead. Replace calls to indexClass() with calls to tclass(). Remove all uses of 'xts_IndexClassSymbol' in C code, including the macros 'GET_xtsIndexClass' and 'SET_xtsIndexClass'. Opportunistically remove .indexCLASS and tclass attributes from objects created using prior versions of xts. Rename R/indexClass.R to R/tclass.R and man/indexClass.Rd to man/tclass.Rd. See #245.

Mark `indexFormat()` and `indexFormat<-` as deprecated, if only to make their usage easier to find during testing and reverse dependency checks. Remove their S3 methods and call their respective "tformat" functions instead. Replace calls to indexFormat() with calls to tformat(). Remove all uses of 'xts_IndexFormatSymbol' in C code, including the macros 'GET_xtsIndexFormat' and 'SET_xtsIndexFormat'. Opportunistically remove .indexFORMAT attribute from objects created using prior versions of xts. Rename R/indexFormat.R to R/tformat.R. See #245.

Index attributes will no longer be attached to the xts object itself, so warn the user if they pass deprecated arguments to the xts constructor. See #245.

Calling lengthgets() creates a new object, which will not have the same attributes as the original object. Wait to copy the original index attributes to the new index until after lengthgets() is potentially called. See #245.

joshuaulrich · 2019-04-28T15:00:18Z

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.

Calling as.matrix() on an xts object without a dim attribute throws an error because ncol() is called on the xts object. ncol() only works if the object has a dim attribute, while NCOL() also works on vectors. Fixes #294. See #245.

We need an all.equal.xts() method to handle the case where all.equal() is called with one xts object that has index attributes on the object and one that only has them on the index. This can cause unit test failures, and did in the "argo" package. The function only removes the index attributes from either (or both) objects before calling NextMethod(). I avoided the urge to check the attributes attached to the object and index for consistency, and chose what seemed to be a simpler approach. See #245.

Silly bug. Add tests for tclass(), tformat(), and tzone(). See #245.

Check for .indexFORMAT and tformat passed to the constructor via '...'. Warn that .indexFORMAT is deprecated, but still use it. We will raise this to an error in a later version. This means the special handling for Ops.xts() is now handled as part of the more general case. It's also not strictly needed, because Ops.xts() no longer passes .indexFORMAT to the constructor. Remove these two elements from '...' before attaching the remaining elements to the result (as user attributes). See #245.

Check for .indexCLASS and tclass passed to the constructor via '...'. Warn that .indexCLASS is deprecated, but still use it. We will raise this to an error in a later version. Also warn if the tclass attribute on the index is not the same as the tclass argument default value ("POSIXct", "POSIXt"). Current behavior over-rides the index tclass attribute with the default argument value. This seems like a bug, but it may break existing code, so warn before changing or converting to an error. Remove these two elements from '...' before attaching the remaining elements to the result (as user attributes). Remove the indexClass() call from the checkXtsClass() test function, since indexClass() is deprecated. See #245.

Check for .indexTZ and tzone passed to the constructor via '...'. Warn that .indexTZ is deprecated and will be ignored. This is consistent with current behavior, but differs from the other two attributes that can be over-ridden via arguments passed via '...'. Remove these two elements from '...' before attaching the remaining elements to the result (as user attributes). Remove the indexTZ() call from the checkXtsTz() test function, since indexTZ() is deprecated. See #245.

We need to drop any index attributes (i.e deprecated .index* and tformat) passed via '...', but retain any user attributes and standard attributes like dim, dimnames, etc. Note that tclass and tzone are not included in the dropped attributes because they are formal arguments to the constructor, and therefore do not appear in '...'. See #245.

The current behavior ignores the tclass attribute on the index passed to .xts(). This is a bug, but throwing a warning breaks existing packages and causes errors in their CRAN checks. Reinstate the warning after fixing dependencies. See #245.

Missed this change while working on #245. Thanks to Claymore Marshall for catching it!

When e1 and e2 do not have identical indexes, we overwrite them with the result of merge(..., retclass = FALSE). That means they no longer have a 'class' attribute, but they still have their 'index' attribute. Next, we calculate 'e' using the Ops method for 'matrix' objects, which means 'e' won't have an index attribute. Also add default methods for `tclass` and `tclass<-`. These were missed during the updates for #245. Fixes #322.

indexClass() was deprecated when all index attributes were moved from the xts object directly to the index. See #245.

joshuaulrich mentioned this issue Jul 17, 2018

reclass() changes "tclass" attribute #249

Closed

joshuaulrich added a commit that referenced this issue Oct 22, 2018

Warn if deprecated attrs passed to xts()

1e64734

Index attributes will no longer be attached to the xts object itself, so warn the user if they pass deprecated arguments to the xts constructor. See #245.

joshuaulrich added a commit that referenced this issue Apr 24, 2019

Warn if deprecated attrs passed to xts()

d63cf64

Index attributes will no longer be attached to the xts object itself, so warn the user if they pass deprecated arguments to the xts constructor. See #245.

joshuaulrich mentioned this issue Apr 28, 2019

Converting xts with no dims to matrix throws error #294

Closed

This was referenced Apr 28, 2019

Ops.xts on dim-less xts and matrix creates malformed xts objects #295

Closed

Replace direct access to tzone attribute edzer/spacetime#38

Merged

joshuaulrich added a commit that referenced this issue May 12, 2019

Fix tformat extractor

f4e9270

Silly bug. Add tests for tclass(), tformat(), and tzone(). See #245.

joshuaulrich closed this as completed in ea22577 May 12, 2019

joshuaulrich added this to the 0.12-0 milestone Jul 4, 2019

joshuaulrich mentioned this issue Aug 6, 2019

Add option to warn about deprecated attributes #306

Closed

joshuaulrich added a commit that referenced this issue Feb 11, 2020

Change indexTZ() to tzone()

21d9c5a

Missed this change while working on #245. Thanks to Claymore Marshall for catching it!

This was referenced Aug 8, 2020

merge.xts returns object without .indexTZ attribute #143

Closed

[R-Forge #5416] index<-.xts and indexClass<-.xts do not correctly set TZ when value is 'Date' #50

Closed

joshuaulrich added a commit that referenced this issue Oct 23, 2022

Remove indexClass() tests

94769a6

indexClass() was deprecated when all index attributes were moved from the xts object directly to the index. See #245.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move index attributes to index object #245

Move index attributes to index object #245

joshuaulrich commented May 29, 2018

joshuaulrich commented May 29, 2018

joshuaulrich commented Oct 23, 2018

joshuaulrich commented Oct 23, 2018

joshuaulrich commented Apr 28, 2019 •

edited

Loading

Move index attributes to index object #245

Move index attributes to index object #245

Comments

joshuaulrich commented May 29, 2018

joshuaulrich commented May 29, 2018

joshuaulrich commented Oct 23, 2018

geoSpectral

PRISM.forecast

RcppXts

joshuaulrich commented Oct 23, 2018

joshuaulrich commented Apr 28, 2019 • edited Loading

joshuaulrich commented Apr 28, 2019 •

edited

Loading