Skip to content

Commit

Permalink
Fixed bug in Summary.soma_adat() operations
Browse files Browse the repository at this point in the history
- these operations: min(), max(), any(), range(), etc.
- would return the incorrect value due to
  an `as.matrix()` conversion under the hood
- now skips that conversion, trips a warning,
  and carries on
- triggers an error of non-numerics are passed
  as part of the '...' outside of a `soma_adat`,
  just like `Summary.data.frame()`
- fixes SomaLogic#121
  • Loading branch information
stufield committed Apr 24, 2024
1 parent 5de962a commit bda55a1
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 8 deletions.
19 changes: 16 additions & 3 deletions R/groupGenerics.R
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,23 @@ Summary.soma_adat <- function(..., na.rm = FALSE) {
args <- lapply(list(...), function(x) {
if ( is.soma_adat(x) ) {
data.matrix(x[, getAnalytes(x)])
.apts <- getAnalytes(x)
rfu <- x[, .apts]
mode_ok <- vapply(rfu, function(.x) {
is.numeric(.x) || is.complex(.x) || is.logical(.x)
}, NA)
if ( !all(mode_ok) ) {
warning(
"Non-numeric variable(s) detected in `soma_adat` object ",
"where RFU values should be. Removing: ",
.value(names(rfu[, .apts])[!mode_ok]), ".", call. = FALSE
)
}
data.matrix(rfu[, mode_ok])
} else if ( !is.numeric(x) && !is.logical(x) && !is.complex(x) ) {
stop(deparse(.Generic),
" is only defined on a `soma_adat` with all numeric-alike variables.",
call. = FALSE
stop("`", .Generic, "()`",
" is only defined on a `soma_adat` with all numeric-alike variables.",
call. = FALSE
)
} else {
x
Expand Down
38 changes: 33 additions & 5 deletions tests/testthat/test-groupGenerics.R
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ test_that("the `Ops()` group generic generates the expected output", {
expect_equal((adat >= 2566.2)$seq.1234.56, c(FALSE, TRUE, TRUE, FALSE, FALSE, TRUE))
expect_equal((adat <= 2566.2)$seq.1234.56, c(TRUE, FALSE, FALSE, TRUE, TRUE, TRUE))

expect_equal(sum(adat > 3000), 10)
expect_equal(sum(adat < 3000), 8)
expect_equal(sum(adat > 3000), 10L)
expect_equal(sum(adat < 3000), 8L)

# meta ata untouched
ops <- adat + 10
Expand Down Expand Up @@ -209,7 +209,35 @@ test_that("error conditions are triggered for non-numerics in RFU block", {
)

# Summary
#expect_error(
# range(tmp)
#)
expect_warning(
out <- range(tmp),
paste(
"Non-numeric variable(s) detected in `soma_adat` object where",
"RFU values should be. Removing: 'seq.1234.56'"
),
fixed = TRUE
)
expect_equal(out, c(2423.9, 4317.8))
# with bad non-adat expressions via '...'

expect_error(
range(adat, "a"),
"`range()` is only defined on a `soma_adat` with all numeric-alike variables",
fixed = TRUE
)
expect_error(
sum(adat, factor("a")),
"`sum()` is only defined on a `soma_adat` with all numeric-alike variables",
fixed = TRUE
)
expect_error(
max(adat, NULL),
"`max()` is only defined on a `soma_adat` with all numeric-alike variables",
fixed = TRUE
)
expect_error(
min(adat, list(a = 1)),
"`min()` is only defined on a `soma_adat` with all numeric-alike variables",
fixed = TRUE
)
})

0 comments on commit bda55a1

Please sign in to comment.