Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Gillis opened this issue Mar 11, 2020 · 3 comments
Closed

Comments

@Gillis
Copy link

Gillis commented Mar 11, 2020

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
Copy link
Owner

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

@joshuaulrich
Copy link
Owner

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
Copy link
Owner

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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants