Merge pull request #214 from atorus-research/devel

Tplyr 1.3.3: fix all-missing (NA) count target sort warning (#213)

Author: mstackhouse

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: Tplyr
 Title: A Traceability Focused Grammar of Clinical Data Summary
-Version: 1.3.2
+Version: 1.3.3
 Authors@R: 
     c(
     person(given = "Eli",

NEWS.md

@@ -1,3 +1,8 @@
+# Tplyr 1.3.3
+
+## Bug fixes
+- Resolve #213 Fix spurious `no non-missing arguments to max` warning and `-Inf` sort value when a count layer targets an all-missing (`NA`) variable
+
 # Tplyr 1.3.2
 
 ## Internal Changes

R/sort.R

@@ -530,7 +530,8 @@ get_data_order_count <- function(formatted_data, formatted_col_index,
         varn_df <- varn_df %>%
           bind_rows(tibble(
             !!target_var[[1]] := names(missing_count_list),
-            !!varN_name := seq_along(missing_count_list) + max(varn_df[, 2])
+            !!varN_name := seq_along(missing_count_list) +
+              if (nrow(varn_df) == 0) 0 else max(varn_df[, 2])
           ))
       } else {
         varn_df <- varn_df %>%
@@ -590,7 +591,8 @@ get_data_order_count <- function(formatted_data, formatted_col_index,
         fact_df <- fact_df %>%
           bind_rows(tibble(
             !!target_var[[1]] := names(missing_count_list),
-            factor_index = seq_along(missing_count_list) + max(fact_df$factor_index)
+            factor_index = seq_along(missing_count_list) +
+              if (nrow(fact_df) == 0) 0 else max(fact_df$factor_index)
           )) %>%
           distinct(!!target_var[[1]], .keep_all = TRUE)
       } else {
@@ -719,8 +721,11 @@ get_data_order_byvarn <- function(formatted_data, by_varn_df, by_var, by_column_
   by_values <- unlist(formatted_data[, by_column_index])
 
   # Look up the VARN value for each row. Unmatched rows get a default
-  # value that places them at the end.
-  default_sort <- max(unlist(by_varn_df[, 2])) + 1
+  # value that places them at the end. When the target has no non-missing
+  # levels (e.g. an all-NA target variable) by_varn_df is empty, so guard
+  # against max() warning and returning -Inf on an empty set.
+  varn_values <- unlist(by_varn_df[, 2])
+  default_sort <- if (length(varn_values) == 0) 1 else max(varn_values) + 1
   varns <- map_dbl(by_values, function(a_by) {
 
     # Row containing the index and the value

cran-comments.md

@@ -1,16 +1,32 @@
-## Submission 1.3.1
-* Internal refactoring and performance optimizations for nested count layers
-* Bug fixes for nested count sorting, negative number extraction, vignette options handling, difftime attributes, apply_row_masks, and risk difference with NAs
+## Submission 1.3.3
+
+This is a patch release (current CRAN version 1.3.2). It fixes a single bug, with
+no user-facing API change:
+
+* Resolve a spurious "no non-missing arguments to max" warning and an invalid
+  `-Inf` sort value produced when a count layer targets an all-missing (`NA`)
+  variable (#213).
 
 ## Test Environments
 
 * Local macOS (aarch64-apple-darwin20), R 4.5.1
-* GitHub Actions: windows-latest (release), macOS-latest (release), ubuntu-latest (release, devel, oldrel-1)
+* GitHub Actions:
+  * windows-latest (R release)
+  * macOS-latest (R release)
+  * ubuntu-latest and ubuntu-22.04 (R release and R devel)
+
+## R CMD check results
 
-## R CMD CHECK Results
 0 errors | 0 warnings | 1 note
 
 * checking for future file timestamps ... NOTE
   unable to verify current time
 
-This NOTE is a transient network issue and not related to the package.
+This NOTE reflects the local machine's inability to reach a time server and is
+unrelated to the package.
+
+## Reverse dependencies
+
+Tplyr has no strong reverse dependencies (Depends/Imports/LinkingTo). Two
+packages list it under Suggests (clinify, logrx); this patch makes no
+user-facing API change and does not affect them.

tests/testthat/test-sort.R

@@ -375,3 +375,44 @@ test_that("Sorting functions work correctly after refactoring", {
   expect_equal(nrow(b_t3), 3)
   expect_true("ord_layer_1" %in% names(b_t3))
 })
+
+test_that("Count layer with an all-missing (NA) target sorts without warning (#213)", {
+  # Regression test for #213: a group_count() target whose values are entirely
+  # NA previously triggered `no non-missing arguments to max; returning -Inf`
+  # and produced a non-finite (-Inf) ord_layer sort value. The bug lived in the
+  # default `byfactor` path -- sort() drops the all-NA level, leaving an empty
+  # lookup table, so max() was called on an empty set in R/sort.R.
+  adsl <- data.frame(
+    USUBJID = sprintf("S%02d", 1:4),
+    TRT01PN = c(1, 1, 2, 2),
+    DCSREAS = NA_character_,
+    stringsAsFactors = FALSE
+  )
+
+  # Every ord_layer_* sort value (whatever the by-structure) must be finite.
+  all_ord_finite <- function(b) all(is.finite(unlist(b[, grepl("^ord_layer_", names(b)), drop = FALSE])))
+
+  # Default (byfactor) sort -- the affected path. Must build without a warning,
+  # carry finite ordering values, and leave the counts intact (2 per group).
+  t1 <- tplyr_table(adsl, TRT01PN) %>%
+    add_layer(
+      group_count(DCSREAS) %>%
+        set_format_strings(f_str("xx", n))
+    )
+  expect_no_warning(b1 <- build(t1))
+  expect_equal(nrow(b1), 1L)
+  expect_true(all_ord_finite(b1))
+  expect_equal(trimws(b1$var1_1), "2")
+  expect_equal(trimws(b1$var1_2), "2")
+
+  # set_missing_count() relabeling of the NA row must also build cleanly.
+  t2 <- tplyr_table(adsl, TRT01PN) %>%
+    add_layer(
+      group_count(DCSREAS) %>%
+        set_format_strings(f_str("xx", n)) %>%
+        set_missing_count(f_str("xx", n), Missing = NA)
+    )
+  expect_no_warning(b2 <- build(t2))
+  expect_true(all_ord_finite(b2))
+  expect_equal(b2$row_label1, "Missing")
+})