From 0cdead5d6a8e63c6bb40dac8aa2e63108a79115f Mon Sep 17 00:00:00 2001 From: ayogasekaram Date: Wed, 6 Nov 2024 17:19:37 +0000 Subject: [PATCH 1/4] add error message for class "difftime" --- NEWS.md | 1 + R/rlistings_methods.R | 9 +++++++++ tests/testthat/test-listings.R | 16 ++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/NEWS.md b/NEWS.md index f0a3730c..a12a0a8f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,5 @@ ## rlistings 0.2.9.9008 + * Added an error message for listings with variables of "difftime" class. ## rlistings 0.2.9 * Added `truetype` font support based on new `formatters` api, by @gmbecker. diff --git a/R/rlistings_methods.R b/R/rlistings_methods.R index d5ca8d6f..4a4db07a 100644 --- a/R/rlistings_methods.R +++ b/R/rlistings_methods.R @@ -15,6 +15,15 @@ dflt_courier <- font_spec("Courier", 9, 1) #' @export #' @name listing_methods print.listing_df <- function(x, widths = NULL, tf_wrap = FALSE, max_width = NULL, fontspec = NULL, col_gap = 3L, ...) { + + # check classes + column_classes <- lapply(x, function(col) sapply(col, class)) + + # Check if any of the columns contain an element of class "difftime" + if (any(sapply(column_classes, function(col) any(col == "difftime")))) { + stop("Error: Listing contains variables of class 'difftime'. Please convert to character or factor.") + } + cat( toString( matrix_form(x, fontspec = fontspec, col_gap = col_gap), diff --git a/tests/testthat/test-listings.R b/tests/testthat/test-listings.R index 125f7da8..4b33af2e 100644 --- a/tests/testthat/test-listings.R +++ b/tests/testthat/test-listings.R @@ -350,3 +350,19 @@ testthat::test_that("split_into_pages_by_var works as expected", { ) testthat::expect_identical(lsting, lsting_id) }) + +testthat::test_that("appropriate error message returned for 'difftime' class", { + tmp_data <- ex_adae[1:100, ] + class(tmp_data$study_duration_secs) <- "difftime" + + lsting <- as_listing( + tmp_data, + key_cols = c("USUBJID", "AGE"), + disp_cols = "study_duration_secs", + main_title = "title", + main_footer = "foot" + ) %>% + split_into_pages_by_var("SEX", page_prefix = "Patient Subset - Sex") + + testthat::expect_error(print(lsting)) +}) From c992c1925c162296ff7acea8728e6430c2d65c7e Mon Sep 17 00:00:00 2001 From: ayogasekaram Date: Wed, 6 Nov 2024 17:41:18 +0000 Subject: [PATCH 2/4] update spelling --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index a12a0a8f..94cf1c75 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,5 @@ ## rlistings 0.2.9.9008 - * Added an error message for listings with variables of "difftime" class. + * Added an error message for listings with variables of `difftime` class. ## rlistings 0.2.9 * Added `truetype` font support based on new `formatters` api, by @gmbecker. From 6ad2098201c1cfad3da1ec6a7f41161197bfefc7 Mon Sep 17 00:00:00 2001 From: ayogasekaram Date: Fri, 8 Nov 2024 13:05:16 +0000 Subject: [PATCH 3/4] moving class check to as_listing --- R/rlistings.R | 4 ++++ R/rlistings_methods.R | 9 --------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/R/rlistings.R b/R/rlistings.R index 11be12e4..04d43d10 100644 --- a/R/rlistings.R +++ b/R/rlistings.R @@ -168,6 +168,10 @@ as_listing <- function(df, ) } + if (any(sapply(df, inherits, "difftime"))) { + stop("One or more variables in the dataframe have class 'difftime'. Please convert to factor or character.") + } + df <- as_tibble(df) varlabs <- var_labels(df, fill = TRUE) o <- do.call(order, df[key_cols]) diff --git a/R/rlistings_methods.R b/R/rlistings_methods.R index 4a4db07a..d5ca8d6f 100644 --- a/R/rlistings_methods.R +++ b/R/rlistings_methods.R @@ -15,15 +15,6 @@ dflt_courier <- font_spec("Courier", 9, 1) #' @export #' @name listing_methods print.listing_df <- function(x, widths = NULL, tf_wrap = FALSE, max_width = NULL, fontspec = NULL, col_gap = 3L, ...) { - - # check classes - column_classes <- lapply(x, function(col) sapply(col, class)) - - # Check if any of the columns contain an element of class "difftime" - if (any(sapply(column_classes, function(col) any(col == "difftime")))) { - stop("Error: Listing contains variables of class 'difftime'. Please convert to character or factor.") - } - cat( toString( matrix_form(x, fontspec = fontspec, col_gap = col_gap), From 40a4723f871d70eccc4f48653f36e6da9a8a3cd9 Mon Sep 17 00:00:00 2001 From: ayogasekaram Date: Fri, 8 Nov 2024 13:19:49 +0000 Subject: [PATCH 4/4] update test --- tests/testthat/test-listings.R | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-listings.R b/tests/testthat/test-listings.R index 4b33af2e..4f463723 100644 --- a/tests/testthat/test-listings.R +++ b/tests/testthat/test-listings.R @@ -355,14 +355,12 @@ testthat::test_that("appropriate error message returned for 'difftime' class", { tmp_data <- ex_adae[1:100, ] class(tmp_data$study_duration_secs) <- "difftime" - lsting <- as_listing( + testthat::expect_error(as_listing( tmp_data, key_cols = c("USUBJID", "AGE"), disp_cols = "study_duration_secs", main_title = "title", main_footer = "foot" ) %>% - split_into_pages_by_var("SEX", page_prefix = "Patient Subset - Sex") - - testthat::expect_error(print(lsting)) + split_into_pages_by_var("SEX", page_prefix = "Patient Subset - Sex")) })