diff --git a/NEWS.md b/NEWS.md index 6b728c8b..679fa3ce 100644 --- a/NEWS.md +++ b/NEWS.md @@ -44,6 +44,12 @@ ## NEW FEATURES 1. `anyNA` gets an `integer64` method. Thanks @hcirellu. +1. The `seq()` method for `integer64` has been overhauled to better match features from the default method. + - The motivation is #47, where `seq(as.integer64(1L), 11L, length.out=6L)` calculated `by=` incorrectly to give `1:6` instead of `c(1L, 3L, ..., 9L, 11L)`. + - `length.out=` was also sometimes ignored, for example `seq(to=as.integer64(5L), length.out=0L)` will now always just give `integer64()`. + - `seq(a, a, by=by)` is no longer an error. + - We match the default method behavior of assuming `from=1` and `to=1` if needed in order to support usage like `seq(as.integer64(10L), by=-1L)` and `seq(by=as.integer64(3L), length.out=8L)`. + - `seq(a, a, length.out=n)` will give `rep(a, n)`, not `seq(a, by=1, length.out=n)`. ## BUG FIXES diff --git a/R/integer64.R b/R/integer64.R index 6b2f3349..add1b9e7 100644 --- a/R/integer64.R +++ b/R/integer64.R @@ -301,37 +301,6 @@ NULL #' @name rep.integer64 NULL -#' integer64: Sequence Generation -#' -#' Generating sequence of integer64 values -#' -#' @param from integer64 scalar (in order to dispatch the integer64 method of [seq()]) -#' @param to scalar -#' @param by scalar -#' @param length.out scalar -#' @param along.with scalar -#' @param ... ignored -#' @details -#' `seq.integer64` does coerce its arguments 'from', 'to' and 'by' to `integer64`. -#' If not provided, the argument 'by' is automatically determined as `+1` or `-1`, -#' but the size of 'by' is not calculated as in [seq()] (because this might result -#' in a non-integer value). -#' -#' @returns an integer64 vector with the generated sequence -#' @note -#' In base R [`:`] currently is not generic and does not dispatch, see section -#' "Limitations inherited from Base R" in [integer64()] -#' -#' @keywords classes manip -#' @seealso [c.integer64()] [rep.integer64()] -#' [as.data.frame.integer64()] [integer64()] -#' @examples -#' # colon not activated: as.integer64(1):12 -#' seq(as.integer64(1), 12, 2) -#' seq(as.integer64(1), by=2, length.out=6) -#' @name seq.integer64 -NULL - #' integer64: Coercing to data.frame column #' #' Coercing integer64 vector to data.frame. @@ -1029,50 +998,105 @@ rep.integer64 <- function(x, ...) { ret } -#' @export -seq.integer64 <- function(from=NULL, to=NULL, by=NULL, length.out=NULL, along.with=NULL, ...) { - if (is.null(length.out)) - length.out <- length(along.with) - else - length.out <- as.integer(length.out) - - if (is.null(by)) { - if (is.null(from) || is.null(to)) - by <- as.integer64(1L) - else - by <- as.integer64(if (to < from) -1L else 1L) - } else { - by <- as.integer64(by) - if (!is.null(from) && !is.null(to) && (sign(by) != (if (to < from) -1L else 1L))) - stop("wrong sign of 'by' argument") +#' Generating sequence of integer64 values +#' +#' @param from integer64 scalar (in order to dispatch the integer64 method of [seq()]) +#' @param to scalar +#' @param by scalar +#' @param length.out scalar +#' @param along.with scalar +#' @param ... ignored +#' @details +#' `seq.integer64` coerces its arguments `from`, `to`, and `by` to `integer64`. Consistency +#' with [seq()] is typically maintained, though results may differ when mixing `integer64` and +#' `double` inputs, for the same reason that any arithmetic with these mixed types can be +#' ambiguous. Whereas `seq(1L, 10L, length.out=8L)` can back up to double storage to give an +#' exact result, this not possible for generic inputs `seq(i64, dbl, length.out=n)`. +#' +#' @returns An integer64 vector with the generated sequence +#' +#' @keywords classes manip +#' @seealso [c.integer64()] [rep.integer64()] +#' [as.data.frame.integer64()] [integer64()] +#' @examples +#' seq(as.integer64(1), 12, 2) +#' seq(as.integer64(1), by=2, length.out=6) +#' +#' # truncation rules +#' seq(as.integer64(1), 10, by=1.5) +#' seq(as.integer64(1), 10, length.out=5) +#' @export +seq.integer64 = function(from=NULL, to=NULL, by=NULL, length.out=NULL, along.with=NULL, ...) { + if (!is.null(along.with)) return(seq.integer64(from, to, by=by, length.out=length(along.with))) + + n_args = 4L - is.null(from) - is.null(to) - is.null(by) - is.null(length.out) + + if (n_args == 4L) + stop("too many arguments") + + if (n_args == 1L) { + one = as.integer64(1L) + if (!is.null(from)) return(one:from) + if (!is.null(to)) return(one:to) + if (!is.null(length.out)) { + if (length.out < 0L) + stop("'length.out' must be a non-negative number") + if (length.out == 0L) + return(integer64()) + return(one:length.out) + } + # match seq(by=integer(1)) + return(one) } - if (is.null(from)) { - if (length.out && length(to)) - from <- to - (length.out-1L)*by - else - from <- as.integer64(1L) - } else { - from <- as.integer64(from) + if (n_args == 2L) { + if (!is.null(length.out)) { + if (length.out == 0L) + return(integer64()) + if (length.out < 0L) + stop("'length.out' must be a non-negative number") + # do before mixing with from/to to avoid integer64/double fraction arithmetic + if (is.double(length.out) && length.out %% 1L != 0L) + length.out = ceiling(length.out) + if (!is.null(from)) + return(seq.integer64(from, from+length.out-1L, by=1L)) + if (!is.null(to)) + return(seq.integer64(to-length.out+1L, to, by=1L)) + if (!is.null(by)) + return(seq.integer64(as.integer64(1L), by=by, length.out=length.out)) + } + if (!is.null(from) && !is.null(to)) return(seq.integer64(from, to, by=sign(to - from))) + if (!is.null(from) && !is.null(by)) return(seq.integer64(from, 1L, by=by)) + return(seq.integer64(as.integer64(1L), to, by=by)) } - if (!length(to)) { - if (length.out) - to <- from + (length.out-1L)*by - else - stop("not enough information provided") - } + # match base behavior for seq(1, 2, length.out=1.5) + if (!is.null(length.out) && is.double(length.out)) + length.out = ceiling(length.out) - if (!length.out) { - length.out <- (to-from) %/% by + 1L - } + if (!is.null(by) && !is.integer64(by)) + by = as.integer64(by) - if (!length.out) return(integer64()) - if (length.out==1L) return(from) - #return(cumsum(c(from, rep(by, length.out-1L)))) - ret <- .Call(C_seq_integer64, from, by, double(as.integer(length.out))) + if (is.null(from)) { + from = to - (length.out - 1L) * by + } else if (is.null(by)) { + if (length.out == 1L) + return(as.integer64(from)) + by = as.integer64((to - from) / (length.out - 1L)) + } else if (is.null(length.out)) { + if (to != from && by == 0L) + stop("invalid '(to - from)/by'") + if (to == from) + return(as.integer64(from)) + if (sign(to - from) != sign(by)) + stop("wrong sign in 'by' argument'") + length.out = (to - from) / by + 1L + } + if (length.out < 0L) + stop("'length.out' must be a non-negative number") + ret <- .Call(C_seq_integer64, as.integer64(from), by, double(as.integer(length.out))) oldClass(ret) <- "integer64" - return(ret) + ret } #' @rdname xor.integer64 diff --git a/man/seq.integer64.Rd b/man/seq.integer64.Rd index 05ea2070..a44eca5a 100644 --- a/man/seq.integer64.Rd +++ b/man/seq.integer64.Rd @@ -2,7 +2,17 @@ % Please edit documentation in R/integer64.R \name{seq.integer64} \alias{seq.integer64} -\title{integer64: Sequence Generation} +\title{Generating sequence of integer64 values} +\usage{ +\method{seq}{integer64}( + from = NULL, + to = NULL, + by = NULL, + length.out = NULL, + along.with = NULL, + ... +) +} \arguments{ \item{from}{integer64 scalar (in order to dispatch the integer64 method of \code{\link[=seq]{seq()}})} @@ -17,25 +27,25 @@ \item{...}{ignored} } \value{ -an integer64 vector with the generated sequence +An integer64 vector with the generated sequence } \description{ Generating sequence of integer64 values } \details{ -\code{seq.integer64} does coerce its arguments 'from', 'to' and 'by' to \code{integer64}. -If not provided, the argument 'by' is automatically determined as \code{+1} or \code{-1}, -but the size of 'by' is not calculated as in \code{\link[=seq]{seq()}} (because this might result -in a non-integer value). -} -\note{ -In base R \code{\link{:}} currently is not generic and does not dispatch, see section -"Limitations inherited from Base R" in \code{\link[=integer64]{integer64()}} +\code{seq.integer64} coerces its arguments \code{from}, \code{to}, and \code{by} to \code{integer64}. Consistency +with \code{\link[=seq]{seq()}} is typically maintained, though results may differ when mixing \code{integer64} and +\code{double} inputs, for the same reason that any arithmetic with these mixed types can be +ambiguous. Whereas \code{seq(1L, 10L, length.out=8L)} can back up to double storage to give an +exact result, this not possible for generic inputs \code{seq(i64, dbl, length.out=n)}. } \examples{ - # colon not activated: as.integer64(1):12 - seq(as.integer64(1), 12, 2) - seq(as.integer64(1), by=2, length.out=6) +seq(as.integer64(1), 12, 2) +seq(as.integer64(1), by=2, length.out=6) + +# truncation rules +seq(as.integer64(1), 10, by=1.5) +seq(as.integer64(1), 10, length.out=5) } \seealso{ \code{\link[=c.integer64]{c.integer64()}} \code{\link[=rep.integer64]{rep.integer64()}} diff --git a/tests/testthat/test-integer64.R b/tests/testthat/test-integer64.R index cee8046f..538db9a1 100644 --- a/tests/testthat/test-integer64.R +++ b/tests/testthat/test-integer64.R @@ -294,12 +294,156 @@ test_that("vector builders of integer64 work", { expect_identical(x[1L]:x[3L], x) expect_identical(x[3L]:x[1L], x[3:1]) # rev() a separate method +}) + +with_parameters_test_that( + "seq method works analogously to integer: 1 argument (except along.with);", + { + n64 = as.integer64(n) + expect_identical(seq(n64), as.integer64(seq(n))) + expect_identical(seq(from=n64), as.integer64(seq(from=n))) + expect_identical(seq(to=n64), as.integer64(seq(to=n))) + expect_identical(seq(by=n64), as.integer64(seq(by=n))) + if (n < 0L) { + expect_error(seq(length.out=n64), "'length.out' must be a non-negative number") + } else { + expect_identical(seq(length.out=n64), as.integer64(seq(length.out=n))) + } + }, + n = c(0L, 5L, -1L) +) + +with_parameters_test_that( + "seq method works analogously to integer: 2 arguments (except along.with)", + { + # be sure to coerce (truncate) consistently across actual/expected + n1_32 = as.integer(n1) + n2_32 = as.integer(n2) + + n1_64 = as.integer64(n1) + n2_64 = as.integer64(n2) + + # TODO(#211): restore parity to seq() here + if (n2 %% 1L == 0L) expect_identical(seq(n1_64, n2), as.integer64(seq(n1_32, n2))) + expect_identical(seq(n1_64, n2_64), as.integer64(seq(n1_32, n2_32))) + + + if (n2 == 0L) { + err_msg = "invalid '(to - from)/by'" + expect_error(seq(n1_64, by=n2), err_msg, fixed=TRUE) + expect_error(seq(to=n1_64, by=n2), err_msg, fixed=TRUE) + } else if (sign(1L - n1) == sign(n2)) { + if (n2 %% 1L == 0L) expect_identical(seq(n1_64, by=n2), as.integer64(seq(n1, by=n2))) + expect_identical(seq(n1_64, by=n2_64), as.integer64(seq(n1, by=n2_32))) + + expect_error(seq(to=n1_64, by=n2), "wrong sign in 'by' argument", fixed=TRUE) + } else { + expect_error(seq(n1_64, by=n2), "wrong sign in 'by' argument", fixed=TRUE) + + # TODO(#211): restore parity to seq() here + if (n2 %% 1L == 0L) expect_identical(seq(to=n1_64, by=n2), as.integer64(seq(to=n1, by=n2))) + expect_identical(seq(to=n1_64, by=n2_64), as.integer64(seq(to=n1, by=n2_32))) + } + + if (n2 >= 0L) { + expect_identical(seq(n1_64, length.out=n2), as.integer64(seq(n1_32, length.out=n2))) + expect_identical(seq(n1_64, length.out=n2_64), as.integer64(seq(n1_32, length.out=n2_32))) + + expect_identical(seq(to=n1_64, length.out=n2), as.integer64(seq(to=n1_32, length.out=n2))) + expect_identical(seq(to=n1_64, length.out=n2_64), as.integer64(seq(to=n1_32, length.out=n2_32))) + + expect_identical(seq(by=n1_64, length.out=n2), as.integer64(seq(by=n1_32, length.out=n2))) + expect_identical(seq(by=n1_64, length.out=n2_64), as.integer64(seq(by=n1_32, length.out=n2_32))) + } else { + err_msg = "'length.out' must be a non-negative number" + expect_error(seq(n1_64, length.out=n2), err_msg, fixed=TRUE) + expect_error(seq(to=n1_64, length.out=n2), err_msg, fixed=TRUE) + expect_error(seq(by=n1_64, length.out=n2), err_msg, fixed=TRUE) + } + }, + .cases = expand.grid(n1=c(0L, 5L, -1L), n2=c(0.0, 5.0, -1.0, 1.5)) +) + +with_parameters_test_that( + "seq method works analogously to integer: 3 arguments (except along.with)", + { + # be sure to coerce (truncate) consistently across actual/expected + n1_32 = as.integer(n1) + n2_32 = as.integer(n2) + n3_32 = as.integer(n3) + + n1_64 = as.integer64(n1) + n2_64 = as.integer64(n2) + n3_64 = as.integer64(n3) + + # TODO(#211): restore parity to seq() here + if (n2 == n1 || sign(n2 - n1) == sign(n3)) { + # TODO(#211): restore parity to seq() here + if (n2 %% 1L == 0L && n3 %% 1L == 0L) { + expect_identical(seq(n1_64, n2, by=n3), as.integer64(seq(n1_32, n2, by=n3))) + expect_identical(seq(n1_64, n2_64, by=n3), as.integer64(seq(n1_32, n2_32, by=n3))) + expect_identical(seq(n1_64, n2, by=n3_64), as.integer64(seq(n1_32, n2, by=n3_32))) + expect_identical(seq(n1_64, n2_64, by=n3_64), as.integer64(seq(n1_32, n2_32, by=n3_32))) + } + } else { + err_msg <- if (n3 == 0L) "invalid '(to - from)/by'" else "wrong sign in 'by' argument" + expect_error(seq(n1_64, n2, by=n3), err_msg, fixed=TRUE) + } + + if (n3 < 0L) { + err_msg = "'length.out' must be a non-negative" + expect_error(seq(n1_64, n2, length.out=n3), err_msg, fixed=TRUE) + expect_error(seq(n1_64, by=n2, length.out=n3), err_msg, fixed=TRUE) + expect_error(seq(to=n1_64, by=n2, length.out=n3), err_msg, fixed=TRUE) + } else { + # TODO(#211): restore parity to seq() here + if (((n2 - n1) / (n3 - 1L)) %% 1L == 0L) { + expect_identical(seq(n1_64, n2, length.out=n3), as.integer64(seq(n1_32, n2, length.out=n3))) + expect_identical(seq(n1_64, n2_64, length.out=n3), as.integer64(seq(n1_32, n2_32, length.out=n3))) + expect_identical(seq(n1_64, n2, length.out=n3_64), as.integer64(seq(n1_32, n2, length.out=n3_32))) + expect_identical(seq(n1_64, n2_64, length.out=n3_64), as.integer64(seq(n1_32, n2_32, length.out=n3_32))) + } + + # TODO(#211): restore parity to seq() here + if (n2 %% 1L == 0L) { + expect_identical(seq(n1_64, by=n2, length.out=n3), as.integer64(seq(n1_32, by=n2, length.out=n3))) + expect_identical(seq(n1_64, by=n2_64, length.out=n3), as.integer64(seq(n1_32, by=n2_32, length.out=n3))) + expect_identical(seq(n1_64, by=n2, length.out=n3_64), as.integer64(seq(n1_32, by=n2, length.out=n3_32))) + expect_identical(seq(n1_64, by=n2_64, length.out=n3_64), as.integer64(seq(n1_32, by=n2_32, length.out=n3_32))) + + expect_identical(seq(to=n1_64, by=n2, length.out=n3), as.integer64(seq(to=n1_32, by=n2, length.out=n3))) + expect_identical(seq(to=n1_64, by=n2_64, length.out=n3), as.integer64(seq(to=n1_32, by=n2_32, length.out=n3))) + expect_identical(seq(to=n1_64, by=n2, length.out=n3_64), as.integer64(seq(to=n1_32, by=n2, length.out=n3_32))) + expect_identical(seq(to=n1_64, by=n2_64, length.out=n3_64), as.integer64(seq(to=n1_32, by=n2_32, length.out=n3_32))) + } + } + }, + .cases = expand.grid(n1=c(0L, 5L, -1L), n2=c(0.0, 5.0, -1.0, 1.5), n3=c(0.0, 5.0, -1.0, 1.5)) +) + +test_that("seq method works analogously to integer: 4 arguments", { + n = as.integer64(5L) + + expect_error(seq(n, n, by=n, length.out=n), "too many arguments") + expect_error(seq(n, n, by=n, along.with=n), "too many arguments") +}) + +test_that("seq() works back-compatibly w.r.t. mixed integer+double inputs", { + one = as.integer64(1L) + expect_identical(seq(one, 10L, by=1.5), as.integer64(1:10)) + expect_identical(seq(to=one, from=10L, by=-1.5), as.integer64(10:1)) + + expect_identical(seq(one, 10L, by=10.0/3.0), as.integer64(c(1L, 4L, 7L, 10L))) + expect_identical(seq(to=one, from=10L, by=-10.0/3.0), as.integer64(c(10L, 7L, 4L, 1L))) + + expect_error(seq(one, 10L, by=0.1), "invalid '(to - from)/by'", fixed=TRUE) + expect_error(seq(to=one, from=10L, by=-0.1), "invalid '(to - from)/by'", fixed=TRUE) + + expect_identical(seq(one, 2.5), as.integer64(1:2)) + expect_identical(seq(to=one, from=2.5), as.integer64(2:1)) - expect_identical(seq(x[1L], x[3L], by=1L), x) - expect_identical(seq(x[1L], x[3L], by=x[1L]), x) - expect_identical(seq(x[1L], to=10L, by=1L), as.integer64(1:10)) - expect_identical(seq(x[1L], to=11L, by=2L), as.integer64(c(1L, 3L, 5L, 7L, 9L, 11L))) - # TODO(#47): More tests when the behavior is corrected. + expect_identical(seq(one, 5.5, by=1.5), as.integer64(1:5)) + expect_identical(seq(to=one, from=5.5, by=-1.5), as.integer64(5:1)) }) # These tests were previously kept as tests under \examples{\dontshow{...}}.