Skip to content

Commit

Permalink
Closes #3014, new warning for %between%
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Chirico committed Aug 29, 2018
1 parent 8d4ce7a commit 0037471
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ Warning message:

4. `hour`, `minute`, and `second` utility functions use integer arithmetic when the input is already (explicitly) UTC-based `POSIXct` for 4-10x speedup vs. using `as.POSIXlt`.

5. Helpful warning added when potentially incorrect usage of `%between%` is detected, [#3014](https://github.com/Rdatatable/data.table/issues/3014). Thanks @peterlittlejohn for offering his user experience and providing the impetus.

### Changes in v1.11.4 (on CRAN 27 May 2018)

Expand Down
13 changes: 12 additions & 1 deletion R/between.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,18 @@ between <- function(x,lower,upper,incbounds=TRUE) {
}

# %between% is vectorised, #534.
"%between%" <- function(x, y) between(x, y[[1L]], y[[2L]], incbounds=TRUE)
"%between%" <- function(x, y) {
if (length(y) > 2L) {
ysub = substitute(y)
warning("RHS has length() greater than 2. ",
if (is.call(ysub) && ysub[[1L]] == 'c')
sprintf("Perhaps you meant %s? ",
capture.output(print(`[[<-`(ysub, 1L, quote(list))))),
"The first element should be the lower bound(s); ",
"the second element should be the upper bound(s).")
}
between(x, y[[1L]], y[[2L]], incbounds=TRUE)
}
# If we want non inclusive bounds with %between%, just +1 to the left, and -1 to the right (assuming integers)

# issue FR #707
Expand Down
7 changes: 7 additions & 0 deletions inst/tests/tests.Rraw
Original file line number Diff line number Diff line change
Expand Up @@ -12134,6 +12134,13 @@ test(1925.12, as.ITime(x, ms='ceil'), structure(c(13L, 68L), class="ITime"))
test(1936.1, fread("A,B\n1,3\n2,4", autostart=1), data.table(A=1:2, B=3:4), warning="autostart.*deprecated.*Consider skip")
if (.Platform$OS.type == "unix") test(1936.2, is.data.table(fread("ls .")))

# add helpful warning to %between%
d <- data.table(A=1:10, B = 3)
test(1937.1, d[A %between% c(B,B+1)], data.table(A = 3L, B = 3),
warning = 'RHS has length().*Perhaps you meant')
test(1937.2, d[A %between% B], data.table(A = 3L, B = 3),
warning = 'greater than 2. The first')

###################################
# Add new tests above this line #
###################################
Expand Down

0 comments on commit 0037471

Please sign in to comment.