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

data.table breaks xts first(), last() distpatching #4053

Closed
ethanbsmith opened this issue Nov 17, 2019 · 4 comments · Fixed by #4065
Closed

data.table breaks xts first(), last() distpatching #4053

ethanbsmith opened this issue Nov 17, 2019 · 4 comments · Fixed by #4065
Assignees
Milestone

Comments

@ethanbsmith
Copy link
Contributor

ethanbsmith commented Nov 17, 2019

upgrading to data.table 12.6 breaks existing xts first(), last() dispatching. Im not sure what the last working verison was, but the broke somewhere between 12.2 and 12.6. reverting to 12.2 restores expected behavior

library(quantmod)
r <- getSymbols("SPY", auto.assign=F)
> last(r)
           SPY.Open SPY.High SPY.Low SPY.Close SPY.Volume SPY.Adjusted
2019-11-15   311.02   311.84  310.26    311.79   62023600       311.79
> library(data.table)
> last(r)
[1] 311.79
@ethanbsmith ethanbsmith changed the title data.table breaks xts first, last data.table breaks xts first(), last() distpatching Nov 17, 2019
@jangorecki
Copy link
Member

jangorecki commented Nov 18, 2019

I am pretty sure it is my fault, made in #3859
I knew it might be breaking change thus asked for revdeps check, but it appears none of the revdeps was testing that. Thanks for reporting.
fyi @joshuaulrich

@jangorecki jangorecki self-assigned this Nov 18, 2019
@jangorecki jangorecki added this to the 1.12.7 milestone Nov 18, 2019
@joshuaulrich
Copy link

Thanks @jangorecki! Let me know if there's anything I can do in xts to help catch things like this in the future, and/or I can do anything to help our two packages play nicely together.

@jangorecki
Copy link
Member

@joshuaulrich extra eyes on a fix PR would be helpful. Thanks

@joshuaulrich
Copy link

Hi Jan, I looked at the PR, and I don't have a good enough understanding of the underlying code to make useful comments.

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

Successfully merging a pull request may close this issue.

4 participants