joshuaulrich / TTR

Technical analysis and other functions to construct technical trading rules with R
GNU General Public License v2.0
325 stars 102 forks source link

cumulative runSD returns all NA #121

Closed ethanbsmith closed 1 year ago

ethanbsmith commented 1 year ago

Description

runSD returns all NA when input has leading NA and cumulative is TRUE

also identified as a regression issue vs. TTR_0.23-4 https://stackoverflow.com/questions/65431269/ttr-runsd-returns-all-na

Expected behavior

expect leading NA to be ignored, like w/ cumulative = FALSE

Minimal, reproducible example

TTR::runSD(c(NA, 1:10), n = 5, cumulative = F)
# [1]       NA       NA       NA       NA       NA 1.581139 1.581139 1.581139 1.581139 1.581139 1.581139
TTR::runSD(c(NA, 1:10), n = 5, cumulative = T)
# [1] NA NA NA NA NA NA NA NA NA NA NA

Session Info

other attached packages:
 [1] rio_0.5.29         rvest_1.0.2        xml2_1.3.3         mlr3verse_0.2.5    mlr3_0.13.4        Rcpp_1.0.9         matrixStats_0.62.0 data.table_1.14.2  quantmod_0.4.20.2  TTR_0.24.3         doFuture_0.12.2   
[12] future_1.27.0      doParallel_1.0.17  iterators_1.0.14   foreach_1.5.2      xts_0.12.1.3       zoo_1.8-10         plotrix_3.8-2 
joshuaulrich commented 1 year ago

I'd appreciate it if you could do some testing on this commit. Thanks for the report!

ethanbsmith commented 1 year ago
> runSD(c(NA, 1:10), n = 1, cumulative = T)
 [1]        NA       NaN 0.7071068 1.0000000 1.2909944 1.5811388 1.8708287 2.1602469 2.4494897 2.7386128 3.0276504
runSD(c(NA, 1:10), n = 1, cumulative = F)
 [1]  NA NaN NaN NaN NaN NaN NaN NaN NaN NaN NaN

produces NaN s in the ouput vs NA. i dont think this is wrong, but getting both NA and NaN seems a bit odd. other than this edge case, its producing the same output as my workaround.

joshuaulrich commented 1 year ago

Thanks for the feedback! I got the first one fixed (cumulative = TRUE). The other one existed in at least one version prior, but I don't like it... so I'm going to make that all NA also.