Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
* Fixed `Aggregate Subject = yes/if-needed` not aggregating reference values, and ratio columns not appearing in results (#1273)

### NCA Results & Export
* Interval parameters (e.g. `AUCINT_0-24`) now display human-readable labels in parameter selectors and boxplot y-axis, instead of raw PPTESTCDs (#1305)
* Descriptive statistics were silently ungrouped when exported before visiting the tab — now falls back to default grouping columns (#1264)
* Fixed NA `PPSTRESU` handling: descriptive statistics no longer crash on all-NA unit groups, and manual interval parameters no longer get `NA` in column names (#1216)
* `get_settings_code()` reads mapping, filters, ratio table, and units from YAML instead of hardcoded defaults (#1189)
Expand Down
13 changes: 13 additions & 0 deletions R/flexible_violinboxplot.R
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,19 @@ flexible_violinboxplot <- function(res_nca,
#' @returns Formatted y-axis label.
#' @noRd
.build_ylabel <- function(parameter, unit) {
# Resolve interval parameter label (e.g. AUCINT_0-24 -> "AUC from 0 to 24")
parsed <- parse_interval_parameter(parameter)
if (parsed$is_interval) {
label <- metadata_nca_parameters$PPTEST[
match(parsed$base, metadata_nca_parameters$PPTESTCD)
]
if (!is.na(label)) {
label <- gsub("T1", as.character(parsed$start), label)
label <- gsub("T2", as.character(parsed$end), label)
parameter <- label
}
}

if (is.null(unit) || is.na(unit) || unit == "" || unit == "unitless") {
parameter
} else {
Expand Down
58 changes: 57 additions & 1 deletion R/label_operators.R
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,47 @@ apply_labels <- function(data, labels_df = metadata_nca_variables, type = "ADNCA
data
}

#' Resolve parameter labels for data frame columns
#'
#' Parses column names like `AUCINT_0-12[Hours*ug/mL]`, strips the unit suffix,
#' resolves the parameter label via `metadata_nca_parameters`, and replaces
#' T1/T2 placeholders with actual interval start/end values.
#'
#' @param data A data frame.
#' @returns Data frame with `label` attributes set for recognized parameter columns.
#' @noRd
resolve_param_labels <- function(data) {
col_names <- names(data)
for (col in col_names) {
# Skip columns that already have a label set
existing_label <- attr(data[[col]], "label")
if (!is.null(existing_label) && !identical(existing_label, col)) next

# Strip unit suffix: "AUCINT_0-12[Hours*ug/mL]" -> "AUCINT_0-12"
pptestcd <- gsub("\\[.*\\]", "", col)
parsed <- parse_interval_parameter(pptestcd)
label <- if (parsed$is_interval) {
base_label <- metadata_nca_parameters$PPTEST[
match(parsed$base, metadata_nca_parameters$PPTESTCD)
]
if (!is.na(base_label)) {
base_label <- gsub("T1", as.character(parsed$start), base_label)
gsub("T2", as.character(parsed$end), base_label)
} else {
NULL
}
} else {
metadata_nca_parameters$PPTEST[
match(pptestcd, metadata_nca_parameters$PPTESTCD)
]
}
if (!is.null(label) && !is.na(label)) {
attr(data[[col]], "label") <- label
}
}
data
}

#' Get the Label of a Heading
#'
#' This function retrieves the label of a heading from a labels file.
Expand Down Expand Up @@ -169,8 +210,23 @@ add_label_attribute <- function(df, myres) {
!is.na(PPSTRESU) & PPSTRESU != "" ~ paste0(PPTESTCD, "[", PPSTRESU, "]"),
TRUE ~ PPTESTCD
),
PPTESTCD_cdisc = translate_terms(PPTESTCD, mapping_col = "PPTESTCD", target_col = "PPTEST")
PPTESTCD_cdisc_raw = translate_terms(
PPTESTCD, mapping_col = "PPTESTCD", target_col = "PPTEST"
),
PPTESTCD_cdisc = PPTESTCD_cdisc_raw
) %>%
rowwise() %>%

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issue 2: rowwise() is slow — use vectorized ifelse() + gsub() instead

rowwise() evaluates each row independently, which is slow on large data frames. This same pattern appears in 3 locations in this PR (R/label_operators.R, R/pivot_wider_pknca_results.R, inst/shiny/functions/selector_label.R).

Since gsub() is already vectorized and start/end are column values, the whole rowwise() %>% mutate() %>% ungroup() block can be replaced with a single vectorized mutate():

Proposed fix (applies to all 3 locations):

# Replace lines 217-229 with:
mutate(
  PPTESTCD_cdisc = ifelse(
    type_interval == "manual",
    gsub("T2", as.character(end),
      gsub("T1", as.character(start), PPTESTCD_cdisc_raw)),
    PPTESTCD_cdisc
  )
) %>%

No rowwise() or ungroup() needed. Same result, fewer lines, better performance.

For selector_label.R, the equivalent would be:

mutate(
  desc = case_when(
    is_interval & !is.na(PPTEST) ~
      gsub("T2", as.character(end_dose),
        gsub("T1", as.character(start_dose), PPTEST)),
    !is.na(PPTEST) ~ PPTEST,
    TRUE ~ PPTESTCD
  )
) %>%

mutate(
PPTESTCD_cdisc = if (type_interval == "manual") {
label <- PPTESTCD_cdisc_raw
label <- gsub("T1", as.character(start), label)
label <- gsub("T2", as.character(end), label)
label
} else {
PPTESTCD_cdisc
}
) %>%
ungroup() %>%
select(PPTESTCD_cdisc, PPTESTCD_unit) %>%
distinct() %>%
pull(PPTESTCD_cdisc, PPTESTCD_unit)
Expand Down
17 changes: 16 additions & 1 deletion R/pivot_wider_pknca_results.R
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,23 @@ add_label_attribute <- function(df, myres) {
!is.na(PPSTRESU) & PPSTRESU != "" ~ paste0(PPTESTCD, "[", PPSTRESU, "]"),
TRUE ~ PPTESTCD
),
PPTESTCD_cdisc = translate_terms(PPTESTCD, mapping_col = "PPTESTCD", target_col = "PPTEST")
PPTESTCD_cdisc_raw = translate_terms(
PPTESTCD, mapping_col = "PPTESTCD", target_col = "PPTEST"
),
PPTESTCD_cdisc = PPTESTCD_cdisc_raw
) %>%
rowwise() %>%
mutate(
PPTESTCD_cdisc = if (type_interval == "manual") {
label <- PPTESTCD_cdisc_raw
label <- gsub("T1", as.character(start), label)
label <- gsub("T2", as.character(end), label)
label
} else {
PPTESTCD_cdisc
}
) %>%

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Issue 1: Duplicated add_label_attribute function

This function is now identical to the one in R/label_operators.R (line 202). Both copies received the same rowwise() + gsub T1/T2 replacement logic in this PR, which increases the maintenance burden — any future change needs to be applied in two places.

Proposed fix: Remove this copy entirely and call the exported version from label_operators.R. Since pivot_wider_pknca_results.R already calls it at line 167, just delete the local definition (lines 213–255) and rely on the exported one:

# In pivot_wider_pknca_results.R, line 167 already does:
pivoted_res <- add_label_attribute(pivoted_res, myres)
# This will resolve to the exported version in R/label_operators.R
# once the local duplicate is removed.

The local copy is marked @noRd @keywords internal, so removing it won't break any public API.

ungroup() %>%
select(PPTESTCD_cdisc, PPTESTCD_unit) %>%
distinct() %>%
pull(PPTESTCD_cdisc, PPTESTCD_unit)
Expand Down
1 change: 1 addition & 0 deletions R/zzz.R
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"PPTEST",
"PPTESTCD",
"PPTESTCD_cdisc",
"PPTESTCD_cdisc_raw",
"PPTESTCD_ref",
"PPTESTCD_unit",
"PPSUMFL",
Expand Down
27 changes: 25 additions & 2 deletions inst/shiny/functions/selector_label.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,35 @@ selector_label <- function(input, output, session,
} else if (metadata_type == "parameter") {
req(metadata_nca_parameters)
choices_df <- data.frame(PPTESTCD = choices, stringsAsFactors = FALSE)

# Parse interval suffix (e.g. AUCINT_0-24 -> base=AUCINT) for label lookup
parsed_info <- lapply(choices_df$PPTESTCD, parse_interval_parameter)
choices_df$base_pptestcd <- vapply(parsed_info, `[[`, "base", FUN.VALUE = "")
choices_df$is_interval <- vapply(parsed_info, `[[`, "is_interval", FUN.VALUE = TRUE)
choices_df$start_dose <- vapply(parsed_info, function(x) {
if (is.null(x$start)) NA_real_ else x$start
}, FUN.VALUE = 0)
choices_df$end_dose <- vapply(parsed_info, function(x) {
if (is.null(x$end)) NA_real_ else x$end
}, FUN.VALUE = 0)

choices_df <- choices_df %>%
left_join(
metadata_nca_parameters %>% select(PPTESTCD, PPTEST) %>% distinct(),
by = "PPTESTCD"
by = c("base_pptestcd" = "PPTESTCD")
) %>%
rowwise() %>%
mutate(
desc = if (is_interval && !is.na(PPTEST)) {
label <- gsub("T1", as.character(start_dose), PPTEST)
gsub("T2", as.character(end_dose), label)
} else if (!is.na(PPTEST)) {
PPTEST
} else {
PPTESTCD
}
) %>%
mutate(desc = ifelse(is.na(PPTEST), PPTESTCD, PPTEST)) %>%
ungroup() %>%
rename(val = PPTESTCD)
} else {
data.frame(val = choices, desc = choices)
Expand Down
2 changes: 1 addition & 1 deletion inst/shiny/modules/common/reactable.R
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ reactable_server <- function(

labeled_data <- reactive({
req(data())
apply_labels(data())
resolve_param_labels(apply_labels(data()))
})

output$table <- renderReactable({
Expand Down
56 changes: 53 additions & 3 deletions tests/testthat/test-label_operators.R
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe("add_label_attribute", {

expect_equal(attr(df_result[["CMAX[ng/mL]"]], "label"), "Max Conc")
expect_equal(attr(df_result[["TMAX[hr]"]], "label"), "Time of CMAX Observation")
expect_equal(attr(df_result[["AUCINT_0-2[hr*ng/mL]"]], "label"), "AUC from T1 to T2")
expect_equal(attr(df_result[["AUCINT_0-2[hr*ng/mL]"]], "label"), "AUC from 0 to 2")
expect_null(attr(df_result[["RandomCol"]], "label"))
})

Expand All @@ -143,7 +143,7 @@ describe("add_label_attribute", {
df_result_mod <- add_label_attribute(df_input_mod, myres_mod)

expect_equal(attr(df_result_mod[["CMAX"]], "label"), "Max Conc")
expect_equal(attr(df_result_mod[["AUCINT_0-2"]], "label"), "AUC from T1 to T2")
expect_equal(attr(df_result_mod[["AUCINT_0-2"]], "label"), "AUC from 0 to 2")
})

it("handles manual intervals with NA PPSTRESU without appending NA to names", {
Expand Down Expand Up @@ -175,7 +175,7 @@ describe("add_label_attribute", {
`TLST[hr]` = "Time of Last Nonzero Conc",
`CLST[ng/mL]` = "Last Nonzero Conc",
`LAMZ[1/hr]` = "Lambda z",
`AUCINT_0-2[hr*ng/mL]` = "AUC from T1 to T2"
`AUCINT_0-2[hr*ng/mL]` = "AUC from 0 to 2"
)

df_input <- as.data.frame(matrix(NA, ncol = length(expected_labels_map), nrow = 1))
Expand All @@ -190,3 +190,53 @@ describe("add_label_attribute", {
))
})
})

describe("resolve_param_labels", {
it("resolves interval column with unit suffix to human-readable label", {
df <- data.frame(x = 1:2)
names(df) <- "AUCINT_0-12[hr*ng/mL]"
result <- resolve_param_labels(df)
expect_equal(
attr(result[["AUCINT_0-12[hr*ng/mL]"]], "label"),
"AUC from 0 to 12"
)
})

it("resolves interval column without unit suffix", {
df <- data.frame(x = 1:2)
names(df) <- "AUCINT_0-24"
result <- resolve_param_labels(df)
expect_equal(
attr(result[["AUCINT_0-24"]], "label"),
"AUC from 0 to 24"
)
})

it("resolves non-interval column to standard PPTEST label", {
df <- data.frame(x = 1:2)
names(df) <- "CMAX"
result <- resolve_param_labels(df)
expect_equal(
attr(result[["CMAX"]], "label"),
"Max Conc"
)
})

it("preserves existing label on column", {
df <- data.frame(x = 1:2)
names(df) <- "AUCINT_0-12"
attr(df[["AUCINT_0-12"]], "label") <- "Custom Label"
result <- resolve_param_labels(df)
expect_equal(
attr(result[["AUCINT_0-12"]], "label"),
"Custom Label"
)
})

it("does not set label for unrecognized column", {
df <- data.frame(x = 1:2)
names(df) <- "UNKNOWN_PARAM"
result <- resolve_param_labels(df)
expect_null(attr(result[["UNKNOWN_PARAM"]], "label"))
})
})
4 changes: 2 additions & 2 deletions tests/testthat/test-pivot_wider_pknca_results.R
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ describe("pivot_wider_pknca_results", {
`LAMZHL[hr]` = "Half-Life Lambda z",
`LAMZSPN` = "Lambda z Span",
`AUCIFO[hr*ng/mL]` = "AUC Infinity Obs",
`AUCINT_0-2[hr*ng/mL]` = "AUC from T1 to T2",
`AUCINT_2-4[hr*ng/mL]` = "AUC from T1 to T2",
`AUCINT_0-2[hr*ng/mL]` = "AUC from 0 to 2",
`AUCINT_2-4[hr*ng/mL]` = "AUC from 2 to 4",
`LAMZIX` = NA, `LAMZMTD` = NA, `Exclude` = NA
)
expect_equal(labels, expected_labels)
Expand Down