fixes coerce from xts where a column x was present #4898
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4898 +/- ##
=======================================
Coverage 99.47% 99.47%
=======================================
Files 75 75
Lines 14789 14844 +55
=======================================
+ Hits 14711 14766 +55
Misses 78 78
Continue to review full report at Codecov.
|
mattdowle
changed the title
| @@ -8,7 +8,7 @@ as.data.table.xts = function(x, keep.rownames = TRUE, key=NULL, ...) { | |||
| if (identical(keep.rownames, FALSE)) return(r[]) | |||
| index_nm = if (is.character(keep.rownames)) keep.rownames else "index" | |||
| if (index_nm %chin% names(x)) stop(domain=NA, gettextf("Input xts object should not have '%s' column because it would result in duplicate column names. Rename '%s' column in xts or use `keep.rownames` to change the index column name.", index_nm, index_nm)) | |||
| r[, c(index_nm) := zoo::index(x)] | |||
| r[, c(index_nm) := zoo::index(x), env=list(x=x)] | |||
There was a problem hiding this comment.
@mattdowle Great to use that you are using new feature. For future I would suggest to apply some styling preferences that will more easily convey which variables are substituted. So far I used to just prefix them with dot.
r[, c(index_nm) := zoo::index(.x), env=list(.x=x)]but I am open for any other alternative. It is not really needed because there is no collision, but for readability it could be useful.
There was a problem hiding this comment.
Yes this is a downside of this approach: the repetition of the variable names. Using .x actually means more like ..x already does and the . vs .. could be confusing; i.e. . conveys current level, and .. conveys one level up, but not here where env= is used if we follow this single . prefix style.
closes #4897
could be resolved more nicely having #4304 merged before