Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix s_summary return for empty logical vectors #1079

Merged
merged 3 commits into from
Oct 10, 2023
Merged

Fix s_summary return for empty logical vectors #1079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41d9796
Fix count_fraction return for empty logical vectors
edelarua Oct 5, 2023
df9261a
Update NEWS
edelarua Oct 5, 2023
918d6fd
Merge branch 'main' into 649_empty_logical_stats@main
edelarua Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

### Enhancements
* Added formatting function `format_count_fraction_lt10` for formatting `count_fraction` with special consideration when count is less than 10.
* Updated `s_summary.logical` output for `count_fraction` when denominator is zero to display as `NA` instead of `0` in tables.

### Miscellaneous
* Began deprecation of `na_level` argument in `s_count_abnormal_by_baseline`, `a_summary`, `analyze_vars`, `analyze_vars_in_cols`, `compare_vars`, `h_map_for_count_abnormal`, `h_stack_by_baskets`, `summarize_colvars`, `a_coxreg`, and `summarize_coxreg` and replaced it with the `na_str` argument.
Expand Down
8 changes: 6 additions & 2 deletions R/analyze_variables.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ s_summary.numeric <- function(x,
#'
#' ## Basic usage:
#' s_summary(factor(c("a", "a", "b", "c", "a")))
#' # Empty factor returns NA-filled items.
#'
#' # Empty factor returns zero-filled items.
#' s_summary(factor(levels = c("a", "b", "c")))
#'
#' ## Management of NA values.
Expand Down Expand Up @@ -382,6 +383,9 @@ s_summary.character <- function(x,
#' ## Basic usage:
#' s_summary(c(TRUE, FALSE, TRUE, TRUE))
#'
#' # Empty factor returns zero-filled items.
#' s_summary(as.logical(c()))
#'
#' ## Management of NA values.
#' x <- c(NA, TRUE, FALSE)
#' s_summary(x, na.rm = TRUE)
Expand Down Expand Up @@ -410,7 +414,7 @@ s_summary.logical <- function(x,
N_col = .N_col
)
y$count <- count
y$count_fraction <- c(count, ifelse(dn > 0, count / dn, NA))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be inf btw ahah

No I was wondering if we want 0 as std here. I think it is better to have NA and then use na_str to set it to NE, right? or I am missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Melkiades, as per Tim's comment on the issue (#649 (comment)) for counts and percentages (3.1.1) "0" should be returned when denominator is zero. This change aligns the logical output with the factor/character output as expected.

y$count_fraction <- c(count, ifelse(dn > 0, count / dn, 0))
y$n_blq <- 0L
y
}
Expand Down
6 changes: 5 additions & 1 deletion man/analyze_variables.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions tests/testthat/_snaps/analyze_variables.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,24 @@
[1] 0


# s_summary works with length 0 logical vectors

Code
res
Output
$n
[1] 0

$count
[1] 0

$count_fraction
[1] 0 0

$n_blq
[1] 0


# s_summary works with logical vectors and by default removes NA

Code
Expand Down
7 changes: 7 additions & 0 deletions tests/testthat/test-analyze_variables.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ testthat::test_that("s_summary works with logical vectors", {
testthat::expect_snapshot(res)
})

testthat::test_that("s_summary works with length 0 logical vectors", {
result <- s_summary(as.logical(c()))

res <- testthat::expect_silent(result)
testthat::expect_snapshot(res)
})

testthat::test_that("s_summary works with logical vectors and by default removes NA", {
x <- c(TRUE, FALSE, TRUE, FALSE, TRUE, TRUE, NA, NA)

Expand Down