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

As of version 0.12 apply.period functions have started reindexing time-variable #330

Closed Gillis closed 4 years ago

Gillis commented 4 years ago

Description

I changed from version 0.11.2 to 0.12, and noted the following new behavior. Because it is not mentioned in the changelog, and does not really make sense to me I believe this is a bug that risks causing all kind of weird bugs for users downstream.

Expected behavior

If you pass an xts indexed by Date into a period.apply function, I would expect it to come out as a Date-indexed one, but as of 0.12 it appears to transform into a posixct. eg.

> t <- xts(order.by = c(as.Date("2020-01-01"),as.Date("2020-02-01")), x = matrix(0,2,2))
> class(index(t))
[1] "Date"
> class(index(apply.daily(t,sum)))
[1] "POSIXct" "POSIXt" 

Minimal, reproducible example

t <- xts(order.by = c(as.Date("2020-01-01"),as.Date("2020-02-01")), x = matrix(0,2,2))
# always gives Date
class(index(t))
# gives posixct under 0.12, date under 0.11.2
class(index(apply.daily(t,sum)))
joshuaulrich commented 4 years ago

Thanks for the report! I can replicate, and will investigate.

joshuaulrich commented 4 years ago

I ran git bisect with this script:

#!/bin/sh

make -B install
Rscript -e 'quit("no", inherits(zoo::index(xts::apply.daily(xts::xts(1:10, .Date(1:10)), sum)), "POSIXt"))'

Which found:

96ecb90d69645d9b64849b9bf69da5625c08cf32 is the first bad commit
commit 96ecb90d69645d9b64849b9bf69da5625c08cf32
Author: Joshua Ulrich <josh.m.ulrich@gmail.com>
Date:   Fri Apr 20 06:46:50 2018 -0500

    Remove .indexCLASS and tclass attrs from xts object

    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.

:100644 100644 a55321ae2c706335bf7d22173133bfb2246bd961 436d1eb901d70aca4e7066a533def1a23875104d M  NAMESPACE
:040000 040000 cf8f9e03cd3d4de0542b5f79bb5312bf9a944ffd 78bafe24e36eb578461bdac388d6a07145527370 M  R
:040000 040000 3e755bc32f1f6cbc5425cd47556b5aadda5ad141 d33a42e0b2127f84d850bc27be9eb9cbaeb6777c M  inst
:040000 040000 3555311029869bc62809edb734b31893de65da90 f15cbc4829b528b4bec72c0c732d3ce69a28ffdb M  man
:040000 040000 4d120f96291576df96e08bf1e79fee0e8e4ee49b 2a95d6c4a1d09dfac8aba5a745f3d6a1ccd0db89 M  src
:040000 040000 8fdf4ac58228844761f7595ad0e34b11b8709cc9 aef1e4e917ad5a6f6f38e585c71b5449b391a6b7 M  vignettes
bisect run success
joshuaulrich commented 4 years ago

This was caused by the issue documented in #322. This issue no longer exists on master, now that the 322 issue branch is merged, so I'm closing this. Thanks again for the report!

R> packageVersion("xts")
[1] '0.12.0.1'
R> t <- xts(order.by = c(as.Date("2020-01-01"),as.Date("2020-02-01")), x = matrix(0,2,2))
R> # always gives Date
R> class(index(t))
[1] "Date"
R> # gives posixct under 0.12, date under 0.11.2
R> class(index(apply.daily(t,sum)))
[1] "Date"