joshuaulrich / xts

Extensible time series class that provides uniform handling of many R time series classes by extending zoo.
http://joshuaulrich.github.io/xts/
GNU General Public License v2.0
220 stars 71 forks source link

window.xts start/end arguments do not support NA #345

Closed harvey131 closed 1 year ago

harvey131 commented 3 years ago

Description

The start and end arguments to window.xts do not support NA the same as if they were NULL.

Inside window_idx() the arguments are passed through .toPOSIXct() which will fail if they are logical (default) NA. However, the start/end arguments are later passed into the index_bsearch() function which does check if start/end are NA.

index_bsearch <- function(index., start, end)
{
  if(!is.null(start) && is.na(start)) return(NULL)
  if(!is.null(end) && is.na(end)) return(NULL)

Expected behavior

Treat start or end NA the same as if they were NULL.

Minimal, reproducible example

 x = xts::xts(1:10, as.POSIXct('2020-1-1') + 1:10)
window(x, start=as.POSIXct('2020-1-1'), end=NA)
# Error in .toPOSIXct(end, tzone(x)) : invalid time / time based class

Session Info

> sessionInfo()
R version 3.6.2 (2019-12-12)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 18363)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252    LC_MONETARY=English_United States.1252 LC_NUMERIC=C                           LC_TIME=English_United States.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
[1] zoo_1.8-7       compiler_3.6.2  xts_0.12-0      grid_3.6.2      lattice_0.20-38
harvey131 commented 3 years ago

Will you accept a pull request to window.xts (via index_bsearch) to modify the behavior to allow NA to the start/end arguments to be interpreted the same as NULL values?

joshuaulrich commented 1 year ago

This is a good suggestion. Would you still be willing to create a PR?

We should add tests to ensure that the behavior of window.xts() matches window.zoo() in these cases. I'd really appreciate it if you added those in the PR.

zeileis commented 1 year ago

I have supported start = NA and end = NA in window.zoo and window<-.zoo on R-Forge now.