Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TESTS: Test scripts risks passing a 'which' out of range to rowOrderStats() #206

Closed
1 task done
HenrikBengtsson opened this issue Jul 1, 2021 · 0 comments
Closed
1 task done
Milestone

Comments

@HenrikBengtsson
Copy link
Owner

HenrikBengtsson commented Jul 1, 2021

Adopted from an off-line report by @AngelPn:

Issue

Test script https://github.com/HenrikBengtsson/matrixStats/blob/develop/tests/rowOrderStats.R:

probs <- runif(1)
which <- round(probs * ncol)
y0 <- rowOrderStats_R(x, probs = probs)
y1 <- rowOrderStats(x, which = which)

and test script https://github.com/HenrikBengtsson/matrixStats/blob/2ef2ab0a308ecc07bf7419ea18ac163df6219d17/tests/rowOrderStats_subset.R:

if (is.null(cols)) which <- round(probs * ncol(x))
else {
xxrows <- rows
suppressWarnings({
xx <- tryCatch(x[, cols, drop = FALSE], error = function(c) "error")
if (identical(xx, "error")) which <- 0
else which <- round(probs * ncol(xx))

tests with a which index that is randomly drawn. However, the way they draw it, risks returning which = 0, which is out of range. It's just by luck from how the initial random seed is set that this is not happening in the current tests.

Action

  • Replace probs <- runif(1); which <- round(probs * max) with a better strategy that guarantees that which is in [1,max].
@HenrikBengtsson HenrikBengtsson added this to the Next release milestone Jul 1, 2021
@HenrikBengtsson HenrikBengtsson modified the milestones: 0.60.0, Next release Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant