search

rkillick / changepoint

A place for the development version of the changepoint package on CRAN.
127 stars 33 forks source link

Checking for 1D objects via `is.null(dim(x))` breaks for 1D-arrays #73

Closed ChristopherEeles closed 1 year ago

ChristopherEeles commented 1 year ago

I noticed this specifically inside of cpt.var, but I imagine it applies to all other places that you check for 1D-ness via is.null(dim(x)).

If you make a vector then coerce it to an array, it gets a dim method. As a result in your checks for for 1D-ness in the package n becomes NULL for 1D- arrays and a missing where TRUE/FALSE needed error is thrown.

A solution would be to add a second check like: is.null(dim(x)) || length(dim(x)) == 1

Reprex

library(changepoint)
v <- rnorm(10)
a <- as.array(v)
cpt.var(v) # works
cpt.var(a)
## Error in if (n < 4) { : missing value where TRUE/FALSE needed

sessionInfo

R version 4.2.0 Patched (2022-04-26 r82266)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 22.04.1 LTS

Matrix products: default
BLAS:   /opt/R/R-latest/lib/R/lib/libRblas.so
LAPACK: /opt/R/R-latest/lib/R/lib/libRlapack.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

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

loaded via a namespace (and not attached):
 [1] zoo_1.8-10        processx_3.7.0    compiler_4.2.0    R6_2.5.1         
 [5] cli_3.3.0         tools_4.2.0       glue_1.6.2        grid_4.2.0       
 [9] changepoint_2.2.3 callr_3.7.0       ps_1.7.0          pak_0.3.0.9000   
[13] lattice_0.20-45
rkillick commented 1 year ago

Thanks for highlighting. Is this something that is a common occurence for you?

I'm happy to add this to the next release but don't feel it requires a patch. If people are coercing data to arrays then the typical use case would be multidimensional. It is easy for the user to add a catch to a batch job for length 1 dimension atttributes and coerce to vector in the meantime.

rkillick commented 1 year ago

Fixed in version 2.2.4 that has been sent to CRAN.

ChristopherEeles commented 1 year ago

Hey Rebecca @rkillick ,

Sorry I must have missed your first reply but thanks for fixing the issue. It was specifically breaking code in the Easy Copy Number package which our lab uses for calling CNAs from several microarray platforms.

Best, Chris