From bda55a16b2fd78141e72b0f288188c4f2f1821e9 Mon Sep 17 00:00:00 2001
From: Stu Field <stu.g.field@gmail.com>
Date: Wed, 24 Apr 2024 10:53:17 -0600
Subject: [PATCH] Fixed bug in `Summary.soma_adat()` operations

- 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 #121
---
 R/groupGenerics.R                   | 19 ++++++++++++---
 tests/testthat/test-groupGenerics.R | 38 +++++++++++++++++++++++++----
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/R/groupGenerics.R b/R/groupGenerics.R
index ff335dc..66e2707 100644
--- a/R/groupGenerics.R
+++ b/R/groupGenerics.R
@@ -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
diff --git a/tests/testthat/test-groupGenerics.R b/tests/testthat/test-groupGenerics.R
index 6d55e7b..a6d8eae 100644
--- a/tests/testthat/test-groupGenerics.R
+++ b/tests/testthat/test-groupGenerics.R
@@ -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
@@ -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
+  )
 })