Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,8 @@

51. `merge.data.table()` silently ignored the `incomparables` argument, [#2587](https://github.com/Rdatatable/data.table/issues/2587). It is now implemented and any other ignored arguments (e.g. misspellings) are now warned about. Thanks to @GBsuperman for the report and @ben-schwen for the fix.

52. `as.xts.data.table` now supports non-numeric xts coredata matrixes, [5268](https://github.com/Rdatatable/data.table/issues/5268). Existing numeric only functionality is supported by a new `numeric.only` parameter, which defaults to `TRUE` for backward compatability and the most common use case. To convert non-numeric columns, set this parameter to `FALSE`. Conversions of `data.table` columns to a `matrix` now uses `data.table::as.matrix`, with all its performance benefits. Thanks to @ethanbsmith for the report and fix.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we plan to support this numeric.only option indefinitely? If not, we should state the migration plan up front.


## NOTES

1. New feature 29 in v1.12.4 (Oct 2019) introduced zero-copy coercion. Our thinking is that requiring you to get the type right in the case of `0` (type double) vs `0L` (type integer) is too inconvenient for you the user. So such coercions happen in `data.table` automatically without warning. Thanks to zero-copy coercion there is no speed penalty, even when calling `set()` many times in a loop, so there's no speed penalty to warn you about either. However, we believe that assigning a character value such as `"2"` into an integer column is more likely to be a user mistake that you would like to be warned about. The type difference (character vs integer) may be the only clue that you have selected the wrong column, or typed the wrong variable to be assigned to that column. For this reason we view character to numeric-like coercion differently and will warn about it. If it is correct, then the warning is intended to nudge you to wrap the RHS with `as.<type>()` so that it is clear to readers of your code that a coercion from character to that type is intended. For example :
Expand Down
13 changes: 8 additions & 5 deletions R/xts.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ as.data.table.xts = function(x, keep.rownames = TRUE, key=NULL, ...) {
r[]
}

as.xts.data.table = function(x, ...) {
as.xts.data.table = function(x, numeric.only = TRUE, ...) {
stopifnot(requireNamespace("xts"), !missing(x), is.data.table(x))
if (!xts::is.timeBased(x[[1L]])) stopf("data.table must have a time based column in first position, use `setcolorder` function to change the order, or see ?timeBased for supported types")
colsNumeric = vapply_1b(x, is.numeric)[-1L] # exclude first col, xts index
if (!all(colsNumeric)) warningf("Following columns are not numeric and will be omitted: %s", brackify(names(colsNumeric)[!colsNumeric]))
r = setDF(x[, .SD, .SDcols = names(colsNumeric)[colsNumeric]])
return(xts::as.xts(r, order.by = if ("IDate" %chin% class(x[[1L]])) as.Date(x[[1L]]) else x[[1L]]))
r <- x[, -1L]# exclude first col, xts index
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the late review. would it be better to shallow()-copy x and then delete the first column?

if (numeric.only) {
colsNumeric = vapply_1b(r, is.numeric)
if (!all(colsNumeric)) warningf("Following columns are not numeric and will be omitted: %s", brackify(names(colsNumeric)[!colsNumeric]))
r <- r[, .SD, .SDcols = names(colsNumeric)[colsNumeric]]
}
return(xts::xts(as.matrix(r), order.by = if (inherits(x[[1L]], "IDate")) as.Date(x[[1L]]) else x[[1L]]))
}
5 changes: 5 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -6861,6 +6861,11 @@ if (test_xts) {
M = xts::as.xts(matrix(1, dimnames=list("2021-05-23", "x"))) # xts:: just to be extra robust; shouldn't be needed with rm(as.xts) above
test(1465.19, inherits(as.data.table(M)$index,"POSIXct"))

# non-numeric xts coredata, #5268
x = xts::xts(x=c(TRUE,FALSE), order.by=Sys.Date()+(1:2))
colnames(x) = "value" # perhaps relates to #4897
test(1465.20, identical(x, as.xts(as.data.table(x), numeric.only=FALSE)))

Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = TRUE)
}

Expand Down
7 changes: 4 additions & 3 deletions man/as.xts.data.table.Rd
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
\alias{as.xts.data.table}
\title{Efficient data.table to xts conversion}
\description{
Efficient conversion of data.table to xts, data.table must have \emph{POSIXct} or \emph{Date} type in first column.
Efficient conversion of data.table to xts, data.table must have a time based type in first column. See ?xts::timeBased for supported types
}
\usage{
as.xts.data.table(x, \dots)
as.xts.data.table(x, numeric.only = TRUE, \dots)
}
\arguments{
\item{x}{data.table to convert to xts, must have \emph{POSIXct} or \emph{Date} in the first column. All others non-numeric columns will be omitted with warning.}
\item{x}{data.table to convert to xts, must have a time based first column. As xts objects are indexed matrixes, all columns must be of the same type. If columns of multiple types are selected, standard as.matrix rules are applied during the conversion. }
\item{numeric.only}{If TRUE, only include numeric columns in the conversion and all non-numeric columns will be omitted with warning}
\item{\dots}{ignored, just for consistency with generic method.}
}
\seealso{ \code{\link{as.data.table.xts}} }
Expand Down