Skip to content

Commit

Permalink
Internal class aware coerceAs. Already used in nafill and froll (#4491)
Browse files Browse the repository at this point in the history
  • Loading branch information
jangorecki authored Mar 8, 2021
1 parent 3fa8b20 commit 85adf09
Show file tree
Hide file tree
Showing 16 changed files with 353 additions and 233 deletions.
11 changes: 11 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,21 @@

## NEW FEATURES

1. `nafill()` now applies `fill=` to the front/back of the vector when `type="locf|nocb"`, [#3594](https://github.com/Rdatatable/data.table/issues/3594). Thanks to @ben519 for the feature request. It also now returns a named object based on the input names. Note that if you are considering joining and then using `nafill(...,type='locf|nocb')` afterwards, please review `roll=`/`rollends=` which should achieve the same result in one step more efficiently. `nafill()` is for when filling-while-joining (i.e. `roll=`/`rollends=`/`nomatch=`) cannot be applied.

## BUG FIXES

## 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 :

```R
x = c(2L,NA,4L,5L)
nafill(x, fill=3) # no warning; requiring 3L too inconvenient
nafill(x, fill="3") # warns in case either x or "3" was a mistake
nafill(x, fill=3.14) # warns that precision has been lost
nafill(x, fill=as.integer(3.14)) # no warning; the as.<type> conveys intent
```

# data.table [v1.14.0](https://github.com/Rdatatable/data.table/milestone/23?closed=1) (submitted to CRAN on 20 Feb 2021)

Expand Down
3 changes: 2 additions & 1 deletion R/data.table.R
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ replace_dot_alias = function(e) {
return(ans)
}
if (!missing(verbose)) {
stopifnot(isTRUEorFALSE(verbose))
if (!is.integer(verbose) && !is.logical(verbose)) stop("verbose must be logical or integer")
if (length(verbose)!=1 || anyNA(verbose)) stop("verbose must be length 1 non-NA")
# set the global verbose option because that is fetched from C code without having to pass it through
oldverbose = options(datatable.verbose=verbose)
on.exit(options(oldverbose))
Expand Down
4 changes: 0 additions & 4 deletions R/shift.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,10 @@ shift = function(x, n=1L, fill=NA, type=c("lag", "lead", "shift"), give.names=FA

nafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA) {
type = match.arg(type)
if (type!="const" && !missing(fill))
warning("argument 'fill' ignored, only make sense for type='const'")
.Call(CnafillR, x, type, fill, nan_is_na(nan), FALSE, NULL)
}

setnafill = function(x, type=c("const","locf","nocb"), fill=NA, nan=NA, cols=seq_along(x)) {
type = match.arg(type)
if (type!="const" && !missing(fill))
warning("argument 'fill' ignored, only make sense for type='const'")
invisible(.Call(CnafillR, x, type, fill, nan_is_na(nan), TRUE, cols))
}
3 changes: 2 additions & 1 deletion R/wrappers.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ fifelse = function(test, yes, no, na=NA) .Call(CfifelseR, test, yes, no, na)
fcase = function(..., default=NA) .Call(CfcaseR, default, parent.frame(), as.list(substitute(list(...)))[-1L])

colnamesInt = function(x, cols, check_dups=FALSE) .Call(CcolnamesInt, x, cols, check_dups)
coerceFill = function(x) .Call(CcoerceFillR, x)

testMsg = function(status=0L, nx=2L, nk=2L) .Call(CtestMsgR, as.integer(status)[1L], as.integer(nx)[1L], as.integer(nk)[1L])

coerceAs = function(x, as, copy=TRUE) .Call(CcoerceAs, x, as, copy)
12 changes: 6 additions & 6 deletions inst/tests/froll.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ test(6000.011, frollmean(x, n, adaptive=TRUE), list(c(NA, 1, 1.25), c(NA, 1, 1.2

#### error on unsupported type
dx = data.table(real=1:10/2, char=letters[1:10])
test(6000.012, frollmean(dx, 3), error="x must be list, data.frame or data.table of numeric or logical types")
test(6000.012, frollmean(dx, 3), error="x must be of type numeric or logical, or a list, data.frame or data.table of such")
dx = data.table(real=1:10/2, fact=factor(letters[1:10]))
test(6000.013, frollmean(dx, 3), error="x must be list, data.frame or data.table of numeric or logical types")
test(6000.013, frollmean(dx, 3), error="x must be of type numeric or logical, or a list, data.frame or data.table of such")
#dx = data.table(real=1:10/2, logi=logical(10))
#test(6000.014, frollmean(dx, 3), error="x must be list, data.frame or data.table of numeric types") # commented out as support added in #3749, tested in .009
dx = data.table(real=1:10/2, list=rep(list(NA), 10))
test(6000.015, frollmean(dx, 3), error="x must be list, data.frame or data.table of numeric or logical types")
test(6000.015, frollmean(dx, 3), error="x must be of type numeric or logical, or a list, data.frame or data.table of such")
x = letters[1:10]
test(6000.016, frollmean(x, 3), error="x must be of type numeric or logical")
test(6000.016, frollmean(x, 3), error="x must be of type numeric or logical, or a list, data.frame or data.table of such")
x = 1:10/2
test(6000.017, frollmean(x, "a"), error="n must be integer")
test(6000.018, frollmean(x, factor("a")), error="n must be integer")
Expand Down Expand Up @@ -355,8 +355,8 @@ test(6000.074, frollmean(1:3, 2, fill=0L), c(0, 1.5, 2.5))
test(6000.075, frollmean(1:3, 2, fill=NA_integer_), c(NA_real_, 1.5, 2.5))
test(6000.076, frollmean(1:3, 2, fill=1:2), error="fill must be a vector of length 1")
test(6000.077, frollmean(1:3, 2, fill=NA), c(NA_real_, 1.5, 2.5))
test(6000.078, frollmean(1:3, 2, fill=TRUE), error="fill must be numeric")
test(6000.079, frollmean(1:3, 2, fill=FALSE), error="fill must be numeric")
test(6000.078, frollmean(1:3, 2, fill=TRUE), frollmean(1:3, 2, fill=1)) #error="fill must be numeric") # fill already coerced, as 'x' arg
test(6000.079, frollmean(1:3, 2, fill=FALSE), frollmean(1:3, 2, fill=0)) #error="fill must be numeric")
test(6000.080, frollmean(1:3, 2, fill="a"), error="fill must be numeric")
test(6000.081, frollmean(1:3, 2, fill=factor("a")), error="fill must be numeric")
test(6000.082, frollmean(1:3, 2, fill=list(NA)), error="fill must be numeric")
Expand Down
Loading

0 comments on commit 85adf09

Please sign in to comment.