Skip to content

Fix: use dose-relative times for manual interval parameter renaming#1169

Open
Gero1999 wants to merge 5 commits intomainfrom
1168-bug/interval-renaming-columns
Open

Fix: use dose-relative times for manual interval parameter renaming#1169
Gero1999 wants to merge 5 commits intomainfrom
1168-bug/interval-renaming-columns

Conversation

@Gero1999
Copy link
Copy Markdown
Collaborator

@Gero1999 Gero1999 commented Apr 1, 2026

Issue

Closes #1168

Description

Two bugs in the descriptive statistics module:

1. Parameter columns not displayed in the table. The parameter selector was refactored from updatePickerInput (raw column names like CMAX[ng/mL]) to selector_label (cleaned names like CMAX). But summary_stats_filtered still used select(any_of(input$select_display_parameters)) — the clean names didn't match the actual column names with units, so any_of() silently dropped all parameter columns.

2. Interval parameter renaming mismatch. Commit 742ca2434 changed signif(start_dose) / signif(end_dose) to start / end in descriptive_statistics.R. This produced absolute interval times instead of dose-relative times, mismatching pivot_wider_pknca_results which uses signif(start_dose) / signif(end_dose). The same incorrect convention was propagated to parameter_plots.R and flexible_violinboxplot.R.

Changes

  • descriptive_statistics.R — map clean parameter names back to actual column names (with units) before selecting; revert interval renaming to signif(start_dose) / signif(end_dose)
  • parameter_plots.R — align interval renaming with pivot_wider_pknca_results
  • flexible_violinboxplot.R — align interval renaming with pivot_wider_pknca_results

How to test

  1. Upload data and run NCA, if possible with multiple profiles
  2. Go to Results > Descriptive Statistics — verify parameter columns (CMAX, AUC, etc.) show numeric values
  3. If manual AUC intervals are configured, verify interval parameters appear correctly (e.g., AUCINT_0-12)
  4. Go to Results > Parameter Plots — verify interval parameters appear in the selector

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • App or package changes are reflected in NEWS
  • Package version is incremented
  • R script works with the new implementation (if applicable)
  • Settings upload works with the new implementation (if applicable)

Gero1999 and others added 5 commits April 1, 2026 09:21
Reverts to signif(start_dose)/signif(end_dose) to match the convention
in pivot_wider_pknca_results. Using start/end produced absolute times
instead of dose-relative times, causing PPTESTCD mismatch.

Fixes #1168

Co-authored-by: Ona <no-reply@ona.com>
Aligns parameter selector choices with pivot_wider_pknca_results
convention.

Co-authored-by: Ona <no-reply@ona.com>
Aligns .prepare_boxplot_data (or inline mutate) with
pivot_wider_pknca_results convention.

Co-authored-by: Ona <no-reply@ona.com>
The parameter selector uses cleaned names (e.g. CMAX) but the
summary_stats columns include units (e.g. CMAX[ng/mL]). The select
was silently dropping all parameter columns via any_of(). Now maps
clean names back to actual column names before selecting.

Co-authored-by: Ona <no-reply@ona.com>
results_to_join selected group/classification columns from conc data
but did not deduplicate. Since conc data has one row per timepoint per
subject, the inner_join duplicated each result row for every timepoint,
inflating the statistics (identical min/max/median within groups).

Co-authored-by: Ona <no-reply@ona.com>
@Shaakon35
Copy link
Copy Markdown
Collaborator

add_label_attribute() in R/pivot_wider_pknca_results.R (line 218) still uses start / end for manual interval naming:

type_interval == "manual" ~ paste0(
  PPTESTCD, "_", start, "-", end,
  ifelse(PPSTRESU != "", paste0("[", PPSTRESU, "]"), "")
)

This is the same inconsistency this PR fixes in the other three files. The column labels generated here would use absolute times while the column names (from pivot_wider_pknca_results) use dose-relative times via signif(start_dose) / signif(end_dose).

Suggested fix:

type_interval == "manual" ~ paste0(
  PPTESTCD, "_", signif(start_dose), "-", signif(end_dose),
  ifelse(PPSTRESU != "", paste0("[", PPSTRESU, "]"), "")
)

Copy link
Copy Markdown
Collaborator

@Shaakon35 Shaakon35 left a comment

Choose a reason for hiding this comment

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

looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: descriptive statistics table shows no parameter values

2 participants