Skip to content

Commit

Permalink
Make create_iso8601() trigger warnings if parsing fails in any of t…
Browse files Browse the repository at this point in the history
…he date/time components

Previously, `create_iso8601()` would not trigger a warning if at least one of the date, time or date-time components parsed successfully. Now it is enough for one single component to fail at parsing for warnings to be triggered. This is following the request: #33 (comment).
  • Loading branch information
ramiromagno committed Jan 17, 2024
1 parent 8d51f5f commit 34e70e2
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 10 deletions.
3 changes: 2 additions & 1 deletion R/dtc_create_iso8601.R
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,8 @@ create_iso8601 <-
cap_matrix <- coalesce_capture_matrices(!!!cap_matrices)

iso8601 <- format_iso8601(cap_matrix, .cutoff_2000 = .cutoff_2000)
iso8601 <- add_problems(iso8601, dots)
any_prob <- any_problems(cap_matrices, .cutoff_2000 = .cutoff_2000)
iso8601 <- add_problems(iso8601, any_prob, dots)
class(iso8601) <- "iso8601"

if (.warn && rlang::is_interactive()) {
Expand Down
24 changes: 17 additions & 7 deletions R/dtc_problems.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
add_problems <- function(x, dtc) {
is_x_na <- is.na(x)
add_problems <- function(x, is_problem, dtc) {
is_x_na <- is_problem
if (!any(is_x_na)) {
return(x)
}
Expand All @@ -16,13 +16,23 @@ add_problems <- function(x, dtc) {

names(dtc) <- names

index <- which(is_x_na)
problems <- tibble::as_tibble(dtc)[is_x_na, ]
index <- which(is_problem)
problems <- tibble::as_tibble(dtc)[is_problem, ]
problems <- tibble::add_column(problems, ..i = index, .before = 1L)
attr(x, "problems") <- problems
x
}

any_problems <- function(cap_matrices, .cutoff_2000 = 68L) {
cap_matrices |>
purrr::map(~ format_iso8601(.x, .cutoff_2000 = .cutoff_2000)) |>
unlist() |>
matrix(ncol = length(cap_matrices)) |>
is.na() |>
rowSums() |>
as.logical()
}

#' Retrieve date/time parsing problems
#'
#' [problems()] is a companion helper function to [create_iso8601()]. It
Expand Down Expand Up @@ -58,8 +68,8 @@ add_problems <- function(x, dtc) {
#' "20231225"
#' )
#'
#' #' # By inspect the problematic dates it can be understood that
#' # the `.format` parameter needs to update to include other variations.
#' #' # By inspecting the problematic dates it can be understood that
#' # the `.format` parameter needs to updated to include other variations.
#' iso8601_dttm <- create_iso8601(dates, .format = "y-m-d")
#' problems(iso8601_dttm)
#'
Expand Down Expand Up @@ -98,7 +108,7 @@ warn_problems <- function(x) {
n_probs <- n_problems(x)
if (n_probs > 0L) {
msg <- paste(
sprintf("There were parsing %d problems.", n_probs),
sprintf("There were %d parsing problems.", n_probs),
"Run `problems()` on parsed results for details."
)
rlang::warn(msg)
Expand Down
4 changes: 2 additions & 2 deletions man/problems.Rd

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

29 changes: 29 additions & 0 deletions tests/testthat/test-create_iso8601.R
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,32 @@ test_that("`create_iso8601()`: dates and times", {
)
expect_identical(as.character(iso8601_dttm), expectation)
})

# https://github.com/pharmaverse/sdtm.oak/pull/33#discussion_r1436195327
test_that("`create_iso8601()`: expect problems", {
dates <- c("999999999", "2000-01-01", "99-01-01", "99-12-31")
times <- c("1520", "0010", "2301", "999999999999")
iso8601_dttm <- create_iso8601(dates, times, .format = c("y-m-d", "HM"), .check_format = FALSE)
expectation <-
structure(
c(
"-----T15:20",
"2000-01-01T00:10",
"1999-01-01T23:01",
"1999-12-31"
),
problems = structure(
list(
..i = c(1L, 4L),
..var1 = c("999999999",
"99-12-31"),
..var2 = c("1520", "999999999999")
),
row.names = c(NA,
-2L),
class = c("tbl_df", "tbl", "data.frame")
),
class = "iso8601"
)
expect_identical(iso8601_dttm, expectation)
})

0 comments on commit 34e70e2

Please sign in to comment.