Skip to content

Commit

Permalink
Ensure endpoints() always returns last observation
Browse files Browse the repository at this point in the history
endpoints() did not return the location of the last observation when
`k > 1` and `on` is "months" or "quarters". This was because the seq()
call did not include the last location.

Create a function to do the check, to keep things DRY.

Thanks to GitHub user Eluvias for the report.

Fixes #300.
  • Loading branch information
joshuaulrich committed Sep 14, 2020
1 parent 352b5a9 commit f7f4a3e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 9 deletions.
23 changes: 17 additions & 6 deletions R/endpoints.R
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ function(x,on='months',k=1) {
if(on %in% c('years','quarters','months','weeks','days'))
posixltindex <- as.POSIXlt(.POSIXct(.index(x)),tz=tzone(x))

include_last <- function(x, k) {
len <- length(x)
i <- seq(1L ,len, k)
if(i[length(i)] != len) {
i <- c(i, len)
}
ep[i]
}

switch(on,
"years" = {
as.integer(c(0, which(diff(posixltindex$year %/% k + 1) != 0), NR))
Expand All @@ -49,16 +58,18 @@ function(x,on='months',k=1) {
ixyear <- posixltindex$year * 100L + 190000L
ixqtr <- ixyear + posixltindex$mon %/% 3L + 1L
ep <- c(0L, which(diff(ixqtr) != 0L), NR)
if(k > 1)
ep[seq(1,length(ep),k)]
else ep
if(k > 1) {
ep <- include_last(ep, k)
}
ep
},
"months" = {
ixmon <- posixltindex$year * 100L + 190000L + posixltindex$mon
ep <- .Call("endpoints", ixmon, 1L, 1L, addlast, PACKAGE='xts')
if(k > 1)
ep[seq(1,length(ep),k)]
else ep
if(k > 1) {
ep <- include_last(ep, k)
}
ep
},
"weeks" = {
.Call("endpoints", .index(x)+3L*86400L, 604800L, k, addlast, PACKAGE='xts')
Expand Down
44 changes: 41 additions & 3 deletions inst/unitTests/runit.endpoints.R
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,46 @@ test.multiple_quarters <- function() {
x <- xts(1:48, as.yearmon("2015-01-01") + 0:47 / 12)
checkIdentical(endpoints(x, "quarters", 1), seq(0L, 48L, 3L))
checkIdentical(endpoints(x, "quarters", 2), seq(0L, 48L, 6L))
checkIdentical(endpoints(x, "quarters", 3), seq(0L, 48L, 9L))
checkIdentical(endpoints(x, "quarters", 3), c(seq(0L, 48L, 9L), 48L))
checkIdentical(endpoints(x, "quarters", 4), seq(0L, 48L,12L))
checkIdentical(endpoints(x, "quarters", 5), seq(0L, 48L,15L))
checkIdentical(endpoints(x, "quarters", 6), seq(0L, 48L,18L))
checkIdentical(endpoints(x, "quarters", 5), c(seq(0L, 48L,15L), 48L))
checkIdentical(endpoints(x, "quarters", 6), c(seq(0L, 48L,18L), 48L))
}

# end(x) always in endpoints(x) result
test.last_obs_always_in_output <- function() {
N <- 341*12
xx <- xts(rnorm(N), seq(Sys.Date(), by = "day", length.out = N))

ep <- endpoints(xx, on = "quarters", k = 2) # OK
checkIdentical(end(xx), end(xx[ep,]), "quarters, k=2")

ep <- endpoints(xx, on = "quarters", k = 3) # NOPE
checkIdentical(end(xx), end(xx[ep,]), "quarters, k=3")

ep <- endpoints(xx, on = "quarters", k = 4) # NOPE
checkIdentical(end(xx), end(xx[ep,]), "quarters, k=4")

ep <- endpoints(xx, on = "quarters", k = 5) # NOPE
checkIdentical(end(xx), end(xx[ep,]), "quarters, k=5")

ep <- endpoints(xx, on = "months", k = 2) # NOPE
checkIdentical(end(xx), end(xx[ep,]), "months, k=2")

ep <- endpoints(xx, on = "months", k = 3) # OK
checkIdentical(end(xx), end(xx[ep,]), "months, k=3")

ep <- endpoints(xx, on = "months", k = 4) # NOPE
checkIdentical(end(xx), end(xx[ep,]), "months, k=4")

# For the "weeks" case works fine

ep <- endpoints(xx, on = "weeks", k = 2) # OK
checkIdentical(end(xx), end(xx[ep,]), "weeks, k=2")

ep <- endpoints(xx, on = "weeks", k = 3) # OK
checkIdentical(end(xx), end(xx[ep,]), "weeks, k=3")

ep <- endpoints(xx, on = "weeks", k = 4) # OK
checkIdentical(end(xx), end(xx[ep,]), "weeks, k=4")
}

0 comments on commit f7f4a3e

Please sign in to comment.