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

Closes #2146 Fix Time Imputation Flag #2195

Merged
merged 12 commits into from
Nov 6, 2023
Merged

Closes #2146 Fix Time Imputation Flag #2195

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
28ebe05
#2146 Fix Time Imputation Flag
ahasoplakus Oct 26, 2023
85498f4
Merge branch 'main' into 2146_compute_tmf_bug
ahasoplakus Oct 26, 2023
4cc6fae
#2146 check templates error
ahasoplakus Oct 26, 2023
ecf1b79
#2146 replace do.call
ahasoplakus Oct 27, 2023
78f9904
Merge branch 'main' into 2146_compute_tmf_bug
ahasoplakus Oct 27, 2023
b1cc2d7
Merge branch '2146_compute_tmf_bug' of https://github.com/pharmaverse…
ahasoplakus Oct 27, 2023
baaaf58
Merge branch 'main' into 2146_compute_tmf_bug
bms63 Oct 31, 2023
f43ff64
Merge branch 'main' into 2146_compute_tmf_bug
ddsjoberg Nov 1, 2023
8bebb26
Replace function short hand with function
ahasoplakus Nov 2, 2023
fe2171f
Merge branch 'main' into 2146_compute_tmf_bug
ahasoplakus Nov 2, 2023
8e682d0
Merge branch 'main' into 2146_compute_tmf_bug
bms63 Nov 3, 2023
3aeb051
Update changelog
ahasoplakus Nov 3, 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
5 changes: 5 additions & 0 deletions NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,12 +201,15 @@ importFrom(lubridate,date)
importFrom(lubridate,days)
importFrom(lubridate,duration)
importFrom(lubridate,floor_date)
importFrom(lubridate,hour)
importFrom(lubridate,hours)
importFrom(lubridate,is.Date)
importFrom(lubridate,is.POSIXct)
importFrom(lubridate,is.instant)
importFrom(lubridate,minute)
importFrom(lubridate,minutes)
importFrom(lubridate,rollback)
importFrom(lubridate,second)
importFrom(lubridate,time_length)
importFrom(lubridate,weeks)
importFrom(lubridate,years)
Expand All @@ -220,6 +223,7 @@ importFrom(purrr,keep)
importFrom(purrr,map)
importFrom(purrr,map2)
importFrom(purrr,map_chr)
importFrom(purrr,map_dbl)
importFrom(purrr,map_if)
importFrom(purrr,map_lgl)
importFrom(purrr,modify_at)
Expand All @@ -244,6 +248,7 @@ importFrom(rlang,current_env)
importFrom(rlang,enexpr)
importFrom(rlang,eval_bare)
importFrom(rlang,eval_tidy)
importFrom(rlang,exec)
importFrom(rlang,expr)
importFrom(rlang,expr_interp)
importFrom(rlang,expr_label)
Expand Down
7 changes: 4 additions & 3 deletions R/admiral-package.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@
#' call2 caller_env call_name current_env .data enexpr eval_bare eval_tidy
#' expr expr_interp expr_label exprs f_lhs f_rhs inform is_call is_expression
#' is_missing new_formula parse_expr parse_exprs set_names sym syms type_of
#' warn as_data_mask list2
#' warn as_data_mask list2 exec
#' @importFrom utils capture.output str file.edit
#' @importFrom purrr map map2 map_chr map_lgl reduce walk keep map_if transpose
#' flatten every modify_at modify_if reduce compose pmap
#' flatten every modify_at modify_if reduce compose pmap map_dbl
#' @importFrom stringr str_c str_count str_detect str_extract str_glue
#' str_length str_locate str_locate_all str_match str_remove str_remove_all
#' str_replace str_replace_all str_split str_starts str_sub str_subset
#' str_trim str_to_lower str_to_title str_to_upper
#' @importFrom lubridate as_datetime ceiling_date date days duration floor_date is.Date is.instant
#' rollback time_length %--% ymd ymd_hms weeks years hours minutes is.POSIXct
#' rollback time_length %--% ymd ymd_hms weeks years hours minutes is.POSIXct hour
#' minute second
#' @importFrom tidyr crossing drop_na fill nest pivot_longer pivot_wider unnest
#' @importFrom tidyselect all_of any_of contains matches vars_select
#' @importFrom hms as_hms
Expand Down
32 changes: 24 additions & 8 deletions R/derive_date_vars.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,25 +1207,41 @@ compute_dtf <- function(dtc, dt) {
#' @export
#'
#' @examples
#' compute_tmf(dtc = "2019-07-18T15:25", dtm = as.POSIXct("2019-07-18T15:25:00"))
#' compute_tmf(dtc = "2019-07-18T15", dtm = as.POSIXct("2019-07-18T15:25:00"))
#' compute_tmf(dtc = "2019-07-18", dtm = as.POSIXct("2019-07-18"))
#' library(lubridate)
#'
#' compute_tmf(dtc = "2019-07-18T15:25", dtm = ymd_hms("2019-07-18T15:25:00"))
#' compute_tmf(dtc = "2019-07-18T15", dtm = ymd_hms("2019-07-18T15:25:00"))
#' compute_tmf(dtc = "2019-07-18", dtm = ymd("2019-07-18"))
#' compute_tmf(dtc = "2022-05--T00:00", dtm = ymd_hms("2022-05-15T23:59:59"))
#' compute_tmf(dtc = "2022-05--T23:00", dtm = ymd_hms("2022-05-15T23:59:59"))
#' compute_tmf(dtc = "2022-05--T23:59:00", dtm = ymd_hms("2022-05-15T23:59:59"))
#'
compute_tmf <- function(dtc,
dtm,
ignore_seconds_flag = FALSE) {
assert_date_vector(dtm)
assert_character_vector(dtc)
assert_logical_scalar(ignore_seconds_flag)

valid_dtc <- is_valid_dtc(dtc)
warn_if_invalid_dtc(dtc, valid_dtc)

partial <- get_partialdatetime(dtc)
highest_miss <- convert_blanks_to_na(vector("character", length(dtc)))
for (c in c("hour", "minute", "second")) {
hms <- c("hour", "minute", "second")
ahasoplakus marked this conversation as resolved.
Show resolved Hide resolved
time_part <-
ahasoplakus marked this conversation as resolved.
Show resolved Hide resolved
map(set_names(hms), \(y) map_dbl(dtm, \(x) exec(y, x)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really like this! It is really clever but certainly has a different look and flavor to rest of our codebase, so I left some recommended comments for the code. In addition, support of the new anonymous function short hand was introduced in purrr 1.0.0, so can you add a change to the DESCRIPTION file to include purrr (>= 1.0.0) ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is an R 4.1 addition, rather than purrr 🐱

Copy link
Collaborator

@zdz2101 zdz2101 Nov 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like a modification to Depends: R (>= 4.1) is necessary instead! I retract the purrr statement

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case, I would just update the line to

map(set_names(hms), function(y) map_dbl(dtm, function(x) exec(y, x)))

so we don't make an R version dependency for something that is not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @zdz2101 @ddsjoberg for the review. I removed the anonymous function shorthand and added the suggested comments.

for (c in hms) {
highest_miss <-
if_else(is.na(partial[[c]]) & is.na(highest_miss), c, highest_miss)
if_else((is.na(partial[[c]]) & is.na(highest_miss)) |
(
!is.na(partial[[c]]) &
is.na(highest_miss) & as.numeric(partial[[c]]) != time_part[[c]]
),
c,
highest_miss
)
}
is_na <- is.na(dtm)
valid_dtc <- is_valid_dtc(dtc)
warn_if_invalid_dtc(dtc, valid_dtc)

map <- c(
hour = "H",
Expand Down
12 changes: 9 additions & 3 deletions man/compute_tmf.Rd
View file
Open in desktop

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

27 changes: 18 additions & 9 deletions tests/testthat/test-derive_date_vars.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,20 +587,26 @@ test_that("compute_tmf Test 30: compute TMF", {
"2003-12-15T-:15:18",
"2003-12-15T13:-:19",
"2020-07--T00:00",
"2020-07--T00:00:00"
"2020-07--T00:00:00",
"2022-05--T00:00",
"2022-05--T23:00",
"2022-05--T23:59:00"
)
input_dtm <- c(
as.POSIXct("2019-07-18T15:25:52"),
as.POSIXct("2019-07-18T15:25:00"),
as.POSIXct("2019-07-18T15:00:00"),
ymd_hms("2019-07-18T15:25:52"),
ymd_hms("2019-07-18T15:25:00"),
ymd_hms("2019-07-18T15:00:00"),
as.POSIXct("2019-07-18"),
as.POSIXct("2019-02-01"),
as.POSIXct(NA_character_),
as.POSIXct(NA_character_),
as.POSIXct("2003-12-15T23:15:18"),
as.POSIXct("2003-12-15T13:59:19"),
as.POSIXct("2020-07-31T00:00:59"),
as.POSIXct("2020-07-31T00:00:59")
ymd_hms("2003-12-15T23:15:18"),
ymd_hms("2003-12-15T13:59:19"),
ymd_hms("2020-07-31T00:00:59"),
ymd_hms("2020-07-31T00:00:00"),
ymd_hms("2022-05-15T23:59:59"),
ymd_hms("2022-05-15T23:59:59"),
ymd_hms("2022-05-15T23:59:59")
)
expected_output <- c(
NA_character_,
Expand All @@ -613,7 +619,10 @@ test_that("compute_tmf Test 30: compute TMF", {
"H",
"M",
"S",
NA_character_
NA_character_,
"H",
"M",
"S"
)

expect_equal(
Expand Down
Loading