Fix bug with assigning p-value name with modpval.tableby. Close #174.

mayoverse · Jan 31, 2019 · 2da7a78 · 2da7a78
1 parent e41eb79
commit 2da7a78
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 3 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,3 +1,9 @@
+# arsenal v2.0.0.9000
+
+* `tableby()` / `paired()`: Fixed two bugs relating to `modpval.tableby()`: one which didn't
+  properly assign the p-value name (#174), and one which broke `as.data.frame()` when
+  assigning custom p-values for only one strata (#175).
+
 # arsenal v2.0.0
 
 There is a new class system (`"arsenal_table"`) which unifies `tableby()`, `modelsum()`, and `freqlist()`.

diff --git a/R/tableby.internal.R b/R/tableby.internal.R
@@ -174,7 +174,7 @@ modpval.tableby <- function(x, pdata, use.pname=FALSE) {
       strat <- if(hasStrata) pdata[[2]][k] else ""
       xname <- pdata[[2 + hasStrata]][k]
       p <- pdata[[3 + hasStrata]][k]
-      method <- if(ncol(pdata) > 3 + hasStrata) pdata[[4 + hasStrata]][k] else "modified by user"
+      method <- if(ncol(pdata) > 3 + hasStrata) pdata[[4 + hasStrata]][k] else "Modified by user"
 
       if(xname %in% names(x$tables[[yname]]$x) && strat %in% x$tables[[yname]]$strata$values)
       {
@@ -185,9 +185,9 @@ modpval.tableby <- function(x, pdata, use.pname=FALSE) {
         x$tables[[yname]]$tables[[idx]][[xname]]$test$method <- method
       }
     }
-    if(use.pname & nchar(names(pdata)[2])>0) {
+    if(use.pname & nchar(names(pdata)[3 + hasStrata]) > 0) {
       ## put different test column name in control
-      x$control$test.pname <- names(pdata)[2]
+      x$control$test.pname <- names(pdata)[3 + hasStrata]
     }
   } else warning("Couldn't match any by-variables to the first column of 'x'.")
   return(x)

diff --git a/tests/testthat/test_lhs_tableby.R b/tests/testthat/test_lhs_tableby.R
@@ -348,3 +348,26 @@ test_that("strata with includeNA()", {
     )
   )
 })
+
+
+####################################################
+
+
+test_that("01/31/2019: modpval.tableby (#174, #175)", {
+  tmp <- tableby(sex ~ age, data = mockstudy, strata = fu.stat, test = FALSE)
+  expect_true(!any(c("test", "p.value") %in% names(as.data.frame(tmp))))
+  tmp <- modpval.tableby(tmp, data.frame(y = "sex", strata = "1", x = "age", p = 1), use.pname = TRUE)
+  expect_identical(
+    capture.kable(summary(tmp, pfootnote = TRUE, text = TRUE)),
+    c("|fu.stat |             |  Male (N=916)   | Female (N=583)  | Total (N=1499)  |    p     |",
+      "|:-------|:------------|:---------------:|:---------------:|:---------------:|:--------:|",
+      "|1       |Age in Years |                 |                 |                 | 1.000^1^ |",
+      "|        |-  Mean (SD) | 58.253 (12.048) | 61.018 (10.649) | 59.336 (11.561) |          |",
+      "|        |-  Range     | 32.000 - 85.000 | 35.000 - 80.000 | 32.000 - 85.000 |          |",
+      "|2       |Age in Years |                 |                 |                 |          |",
+      "|        |-  Mean (SD) | 60.686 (11.278) | 59.059 (11.824) | 60.054 (11.516) |          |",
+      "|        |-  Range     | 19.000 - 88.000 | 22.000 - 88.000 | 19.000 - 88.000 |          |",
+      "1. Modified by user"
+    )
+  )
+})