Skip to content

as.xts.data.table support for all date based classes, #2408#2419

Merged
mattdowle merged 10 commits intoRdatatable:masterfrom
ethanbsmith:master
Oct 15, 2017
Merged

as.xts.data.table support for all date based classes, #2408#2419
mattdowle merged 10 commits intoRdatatable:masterfrom
ethanbsmith:master

Conversation

@ethanbsmith
Copy link
Copy Markdown
Contributor

@ethanbsmith ethanbsmith commented Oct 13, 2017

my first github pull request. apologies if I messed anything up. just trying to help
closes #2408

@mattdowle
Copy link
Copy Markdown
Member

A very warm welcome @ebs238 ! Glad to see @MichaelChirico is replying in the issue so I'll leave it to you. Once the PR is passing I'll look again.

@MichaelChirico
Copy link
Copy Markdown
Member

Hey @ebs238 thanks so much!

You can click the Details link next to either Travis CI or AppVeyor to see an event log. I see this there:

Error in parse(outFile) : 
  /tmp/Rtmpdh5GmW/Rbuild3b0e12324b21/data.table/R/xts.R:16:47: unexpected '='
15:     r = setDF(x[, .SD, .SDcols = names(colsNumeric)[colsNumeric]])
16:     return(xts::as.xts(r, order.by = order.by =
                                                  ^

Looking at your commit diff, indeed we can see you doubled order.by = by accident. Please fix that typo and see if anything else is keeping the tests from passing 😄

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 14, 2017

Codecov Report

Merging #2419 into master will increase coverage by 0.04%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2419      +/-   ##
=========================================
+ Coverage   91.26%   91.3%   +0.04%     
=========================================
  Files          62      62              
  Lines       12027   12027              
=========================================
+ Hits        10976   10981       +5     
+ Misses       1051    1046       -5
Impacted Files Coverage Δ
R/xts.R 66.66% <50%> (ø) ⬆️
src/rbindlist.c 88.93% <0%> (+0.19%) ⬆️
src/forder.c 94.47% <0%> (+0.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70db3e4...f785157. Read the comment docs.

@mattdowle mattdowle added this to the v1.10.6 milestone Oct 15, 2017
@mattdowle mattdowle changed the title fix for #2408 as.xts.data.table support for all date based classes, #2408 Oct 15, 2017
@mattdowle mattdowle merged commit 3e12a39 into Rdatatable:master Oct 15, 2017
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 this pull request may close these issues.

as.xts.data.table does not properly support all date based classes for index

4 participants