[Review view] SDTM Phase 1: R conversion functions + tests#1328
Draft
Gero1999 wants to merge 1 commit into
Draft
[Review view] SDTM Phase 1: R conversion functions + tests#1328Gero1999 wants to merge 1 commit into
Gero1999 wants to merge 1 commit into
Conversation
New R functions to convert CDISC SDTM domain data (PC, EX, DM) into PKNCAdata objects for NCA computation: - sdtm_to_PKNCAdata(): top-level entry point - pc_to_PKNCAconc(): PC domain to PKNCAconc with timing derivations - ex_to_PKNCAdose(): EX domain to PKNCAdose with duration/route parsing - parse_iso8601_duration(): ISO 8601 duration string to numeric hours - route_cdisc_to_pknca(): CDISC route terms to PKNCA route values - std_dtc_to_rdate(): ISO 8601 datetime string to POSIXct Includes 60+ unit tests covering standard usage, edge cases, multi-dose scenarios, metabolite handling, and error conditions. Co-authored-by: Ona <no-reply@ona.com>
This was referenced May 27, 2026
Draft
KOBANAK
reviewed
Jun 4, 2026
Comment on lines
+95
to
+107
| if ("EXDUR" %in% names(ex)) { | ||
| adosedur <- if (is.character(ex$EXDUR)) { | ||
| parse_iso8601_duration(ex$EXDUR) | ||
| } else { | ||
| as.numeric(ex$EXDUR) | ||
| } | ||
| } else if ("EXENDTC" %in% names(ex)) { | ||
| end_posix <- std_dtc_to_rdate(ex$EXENDTC) | ||
| dur <- as.numeric(difftime(end_posix, exstdtc_posix, units = "hours")) | ||
| adosedur <- ifelse(is.na(dur), 0, dur) | ||
| } else { | ||
| adosedur <- rep(0, nrow(ex)) | ||
| } |
Collaborator
There was a problem hiding this comment.
Should we modify the duration calculation logic to prevent crashes when EXDUR column is present but entirely empty (containing only NAs or blank strings). Like instead falling through to EXENDTC which might have actual values ?
Suggested change
| if ("EXDUR" %in% names(ex)) { | |
| adosedur <- if (is.character(ex$EXDUR)) { | |
| parse_iso8601_duration(ex$EXDUR) | |
| } else { | |
| as.numeric(ex$EXDUR) | |
| } | |
| } else if ("EXENDTC" %in% names(ex)) { | |
| end_posix <- std_dtc_to_rdate(ex$EXENDTC) | |
| dur <- as.numeric(difftime(end_posix, exstdtc_posix, units = "hours")) | |
| adosedur <- ifelse(is.na(dur), 0, dur) | |
| } else { | |
| adosedur <- rep(0, nrow(ex)) | |
| } | |
| if ("EXDUR" %in% names(ex) && !all(is.na(ex$EXDUR) | !nzchar(ex$EXDUR))) { | |
| adosedur <- if (is.character(ex$EXDUR)) { | |
| parse_iso8601_duration(ex$EXDUR) | |
| } else { | |
| as.numeric(ex$EXDUR) | |
| } | |
| } else if ("EXENDTC" %in% names(ex)) { | |
| end_posix <- std_dtc_to_rdate(ex$EXENDTC) | |
| adosedur <- as.numeric(difftime(end_posix, exstdtc_posix, units = "hours")) | |
| } else { | |
| adosedur <- rep(0, nrow(ex)) | |
| } | |
| adosedur <- ifelse(is.na(adosedur), 0, adosedur) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a review view into the SDTM changes from #1318. It isolates the R package conversion functions for focused code review. The actual merge will happen via #1318.
Scope
New R functions in
R/sdtm_to_pknca.Rthat convert CDISC SDTM domain data (PC, EX, DM) intoPKNCAdataobjects:sdtm_to_PKNCAdata(pc, ex, dm, metabolites)PKNCAdatapc_to_PKNCAconc(pc, ex, dm)PKNCAconcwith timing derivationsex_to_PKNCAdose(ex)PKNCAdosewith duration/route parsingparse_iso8601_duration(x)route_cdisc_to_pknca(routes)std_dtc_to_rdate(dtc)Plus internal helpers:
.check_required_cols(),.prepare_dose_table(),.assign_doses_to_conc().Files changed
R/sdtm_to_pknca.R— 537 lines, all newNAMESPACE— 4 new exportsman/— 9 new.Rdfilestests/testthat/test-sdtm_to_pknca.R— 908 lines, 60+ testsWhat to review