HenrikBengtsson / matrixStats

R package: Methods that Apply to Rows and Columns of Matrices (and to Vectors)
https://cran.r-project.org/package=matrixStats
203 stars 33 forks source link

Bug in rowCollapse if combined with row subsetting #170

Closed const-ae closed 4 years ago

const-ae commented 4 years ago

Hi Henrik,

I think I found a bug in rowCollapse if it is combined with row subsetting:

mat <- matrix(1:15, nrow = 5, ncol = 3)
print(mat)
#>      [,1] [,2] [,3]
#> [1,]    1    6   11
#> [2,]    2    7   12
#> [3,]    3    8   13
#> [4,]    4    9   14
#> [5,]    5   10   15
matrixStats::rowCollapse(mat, idxs = 2)
#> [1]  6  7  8  9 10
matrixStats::rowCollapse(mat, idxs = 2, rows = 1:4)
#> [1]  6 NA NA NA
matrixStats::rowCollapse(mat, idxs = rep(2, 5), rows = 1:4)
#> [1] 6 7 8 9

Created on 2020-03-19 by the reprex package (v0.3.0) I would expect the last two lines to do the same.

I think the problem is here where the idxs vector is subset. This works well if idxs has nrow(mat) elements, but returns NA if rows is longer than idxs.

If you want, I can draft a quick PR to fix the issue.

HenrikBengtsson commented 4 years ago

Thxs. Fixed for next release.

HenrikBengtsson commented 4 years ago

BTW, this is a good example where full-coverage unit tests != solid system tests. Forgot to test the combination of idxs and subsetting.