Skip to content

Commit

Permalink
Warn when 'i' or 'j' is negative non-integer
Browse files Browse the repository at this point in the history
We need to convert 'i' and 'j' to integer if they're numeric with a
fractional component. This was only being done when 'i' and 'j' were
positive.

Also fix the if-statement. sum() could have returned NA and it's better
to use isTRUE() and any() anyway.

Fixes #415. See #413.
  • Loading branch information
joshuaulrich committed Feb 6, 2024
1 parent 3cf1e39 commit 48cc21a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 13 deletions.
28 changes: 17 additions & 11 deletions R/xts.methods.R
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ function(x, i, j, drop = FALSE, which.i=FALSE,...)
if(!missing(i)) {
# test for negative subscripting in i
if (is.numeric(i)) {

# warn and convert if 'i' is not integer-like
i_int <- as.integer(i)
i_eps <- abs(i) - abs(i_int)
if (isTRUE(any(i_eps > sqrt(.Machine$double.eps)))) {
warning("converting 'i' to integer because it appears to contain fractions")
i <- i_int
}
#if(any(i < 0)) {
if(.Call(C_any_negative, i)) {
if(!all(i <= 0))
Expand All @@ -143,12 +151,6 @@ function(x, i, j, drop = FALSE, which.i=FALSE,...)
if(length(i) > 0 && max(i) > nr)
stop('subscript out of bounds')
#i <- i[-which(i == 0)]

# warn and convert if 'i' is not integer-like
if(sum(abs(i)-abs(as.integer(i))) > .Machine$double.eps) {
warning("converting 'i' to integer because it appears to contain fractions")
i <- as.integer(i)
}
} else
if (timeBased(i) || (inherits(i, "AsIs") && is.character(i)) ) {
# Fast binary search on set of dates
Expand Down Expand Up @@ -237,18 +239,22 @@ function(x, i, j, drop = FALSE, which.i=FALSE,...)
} else
# test for negative subscripting in j
if (is.numeric(j)) {

# warn and convert if 'j' is not integer-like
j_int <- as.integer(j)
j_eps <- abs(j) - abs(j_int)
if (isTRUE(any(j_eps > sqrt(.Machine$double.eps)))) {
warning("converting 'j' to integer because it appears to contain fractions")
j <- j_int
}

if(min(j,na.rm=TRUE) < 0) {
if(max(j,na.rm=TRUE) > 0)
stop('only zeros may be mixed with negative subscripts')
j <- (1:nc)[j]
}
if(max(j,na.rm=TRUE) > nc)
stop('subscript out of bounds')
# warn and convert if 'j' is not integer-like
if(sum(abs(j)-abs(as.integer(j))) > .Machine$double.eps) {
warning("converting 'j' to integer because it appears to contain fractions")
j <- as.integer(j)
}
} else
if(is.logical(j)) {
if(length(j) == 1) {
Expand Down
4 changes: 2 additions & 2 deletions inst/tinytest/test-subset.R
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,10 @@ expect_equal(storage.mode(x[0, tf]), sm, info = paste(info_msg, ": x[0, c(TRUE,
# non-integer subset
x <- .xts(matrix(1:20, 10, 2), 1:10)
# subset by non-integer-like 'i' warns
#expect_warning(x[-1.5, ])
expect_warning(x[-1.5, ])
expect_warning(x[ 0.5, ])
expect_warning(x[ 1.5, ])
# subset by non-integer-like 'j' warns
#expect_warning(x[, -1.5])
expect_warning(x[, -1.5])
expect_warning(x[, 0.5])
expect_warning(x[, 1.5])

0 comments on commit 48cc21a

Please sign in to comment.