139 feat/blq options #491

Gero1999 · 2025-06-26T09:32:05Z

Issue

Closes #139

Description

This PR allows users to flexibly specify BLQ imputation rules from the UI and propagate these rules through the NCA data processing pipeline. The changes include both backend logic for applying BLQ imputation and frontend UI updates to let users select their preferred BLQ handling strategy.

BLQ Imputation Feature

Added support for user-defined BLQ imputation rules in NCA setup, enabling PKNCA flexible strategies (e.g., positional, tmax-based, value assignment, or no handling) via the new data_imputation_ui and data_imputation_server modules,
Extended the NCA pipeline to accept and apply BLQ imputation rules, updating relevant functions (update_main_intervals, rm_impute_obs_params)
Updated NEWS.md and function documentation to announce and describe the new BLQ imputation capability.
Registered new global variables and helper functions required for BLQ imputation in the package metadata.

Definition of Done

Note: Considering our latest conversations this feat is only in order to incorporate the options that PKNCA support. But we are not doing it with actual intention of internal use. That said, I would suggest against overcomplicating the feature options far from what PKNCA supports

Allows PKNCA imputation based on tmax, BLQ position or allows to introduce a particular BLQ value (i.e LLOQ/2)
The process affects the NCA results when relevant (missing concentrations!)

How to test

Map data > NCA settings > Test the different BLQ imputations in Data Imputation section

Use extreme value imputations (i.e, 10000) and check if the auclast is changed for profiles with BLQ values. I made modifications in data to include BLQ points (AVAL = 0), one case is the first profile (USUBJID = 11101, PCSPEC = "CSF", NCA_PROFILE = 1)

Contributor checklist

Notes to reviewer

~~Blocked until we figure out what to do with the potential bug in PKNCA!~~ No need

js3110

Hi,

I added some comments, and also I have noticed an issue in the handling of the BLQs, it seems to be dropping the last sample in all the options.

For example, subjid 11101 in the main branch has a slope with points 5, 6, 7, where the last point is time = 383.7, conc = 0.0063.
In this branch, that time point is gone and only 5 and 6 are there, which is not enough for slope calculations,

It seems to be the same trend for all the subjects.
This is not only an issue in the plots but also in the main results.

inst/shiny/modules/tab_nca/setup/data_imputation.R

Gero1999 · 2025-06-27T14:02:44Z

Hi,

I added some comments, and also I have noticed an issue in the handling of the BLQs, it seems to be dropping the last sample in all the options.

For example, subjid 11101 in the main branch has a slope with points 5, 6, 7, where the last point is time = 383.7, conc = 0.0063. In this branch, that time point is gone and only 5 and 6 are there, which is not enough for slope calculations,

It seems to be the same trend for all the subjects. This is not only an issue in the plots but also in the main results.

@js3110 Hey, I indeed changed the Dummy_complex_data.csv so all AVAL<0.05 = BLQ, just for the sake of creating examples that can be tested. So if you use the standard BLQ imputation, all points after tmax that were lower than 0.05 will be drop. This is in theory the right approach! Could it be that the "issue" you observed?

js3110 · 2025-06-27T14:35:49Z

@js3110 Hey, I indeed changed the Dummy_complex_data.csv so all AVAL<0.05 = BLQ, just for the sake of creating examples that can be tested. So if you use the standard BLQ imputation, all points after tmax that were lower than 0.05 will be drop. This is in theory the right approach! Could it be that the "issue" you observed?

I observed that the last point was dropped for all options selected- including when I chose "keep"- so I don't think thats correct?

Gero1999 · 2025-07-01T08:47:27Z

dataset is back to what it used for simplification so lambda can still be calculated for first profile. You can still test the thing thanks to the predoses

Gero1999 · 2025-07-07T06:36:25Z

@js3110 @m-kolomanski Not sure what is the best to do here. Due to the potential discovered bug already reported in PKNCA (here), using a BLQ rule with tmax.before = "drop" or first = "drop" seems to cause the drop of the start_conc0 imputation in extravascular studies.

I opened a discussion in PKNCA to figure out if this is indeed the situation and needs a fix (humanpred/pknca#447) But maybe @billdenney can confirm if this is the issue or if he knows of a workaround for it

js3110 · 2025-07-08T13:47:05Z

@js3110 @m-kolomanski Not sure what is the best to do here. Due to the potential discovered bug already reported in PKNCA (here), using a BLQ rule with tmax.before = "drop" or first = "drop" seems to cause the drop of the start_conc0 imputation in extravascular studies.

I opened a discussion in PKNCA to figure out if this is indeed the situation and needs a fix (humanpred/pknca#447) But maybe @billdenney can confirm if this is the issue or if he knows of a workaround for it

since this issue is not a blocker I would say we can pause it for now and wait until the bugs are fixed!

billdenney · 2025-07-08T21:36:43Z

For the potential bug in PKNCA, is it a real use case where you would say that you want to drop early BLQ values but also you want to impute a BLQ value? It seems like the user is giving discordant instructions at that point.

If it is a real use case (rather than option interaction testing which is useful), can you please describe the scenario for this to be intentional?

Merge remote-tracking branch 'origin/main' into 139-feat/blq-options # Conflicts: # inst/shiny/modules/tab_nca.R # inst/shiny/modules/tab_nca/setup/settings.R

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (2)

man/rm_impute_obs_params.Rd:5

The title "Apply Imputation for PKNCA Data" is misleading for a function now named rm_impute_obs_params. The function's primary purpose is to remove imputation from observational parameters, not to apply imputation. The title should be updated to something like "Remove Imputation from Observational Parameters" to accurately reflect the function's purpose.
man/rm_impute_obs_params.Rd:16
The return value description states "A PKNCAdata object with imputation rules applied" which is misleading since this function removes imputation rules from observational parameters rather than applying them. The description should be updated to "A PKNCAdata object with imputation removed from observational parameters".

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

man/update_main_intervals.Rd

R/intervals.R

inst/shiny/modules/tab_nca/setup/data_imputation.R

inst/shiny/modules/tab_nca/setup.R

R/intervals.R

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

man/rm_impute_obs_params.Rd:5

The function was renamed from apply_imputation to rm_impute_obs_params. However, the title still says "Apply Imputation for PKNCA Data" which no longer accurately reflects the function's purpose. The new name suggests it removes imputation from observational parameters, so the title should be updated to something like "Remove Imputation from Observational Parameters" to match the new function name and behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-02T10:48:48Z

inst/shiny/modules/tab_nca/setup/data_imputation.R

+      ),
+      conditionalPanel(
+        condition = sprintf("input['%s'] == 'Set value for all BLQ'", ns("select_blq_strategy")),
+        blq_selectize(ns("blq_value"), "Value for BLQ", selected = "0.05"),


Trailing comma after the last argument. While R allows trailing commas in function calls, this is inconsistent with the other conditional panels in this file where no trailing comma is used (see lines 37, 45). For consistency, consider removing the trailing comma.

Suggested change

blq_selectize(ns("blq_value"), "Value for BLQ", selected = "0.05"),

blq_selectize(ns("blq_value"), "Value for BLQ", selected = "0.05")

R/intervals.R

Copilot · 2026-01-02T10:48:49Z

R/intervals.R

 #' Apply Imputation for PKNCA Data
 #'
 #' Applies imputation rules to a PKNCAdata object, imputing start values and then
 #' selectively removing imputation for parameters that are not dependent on AUC.


The description states this function "imputing start values and then selectively removing imputation" but this is no longer accurate. After the refactoring, this function only removes imputation from observational parameters (those not dependent on AUC). The start value imputation is now handled separately by create_start_impute. Update the description to accurately reflect that this function only removes imputation rules from parameters that are not dependent on AUC.

Copilot · 2026-01-02T10:48:49Z

inst/shiny/modules/tab_nca/setup/data_imputation.R

+      rule_list <- switch(
+        input$select_blq_strategy,
+        "Tmax based imputation" = list(
+          before.tmax = input$before_tmax,
+          after.tmax = input$after_tmax
+        ),
+        "Positional BLQ imputation" = list(
+          first = input$before_first_non_blq,
+          middle = input$in_between_non_blqs,
+          last = input$after_last_non_blq
+        ),
+        "Set value for all BLQ" = list(
+          first = input$blq_value,
+          middle = input$blq_value,
+          last = input$blq_value
+        ),
+        "No BLQ handling" = list(
+          first = "keep",
+          middle = "keep",
+          last = "keep"
+        ),
+        NULL
+      )


When the user switches between BLQ imputation strategies, the reactive may attempt to access input values that are not yet defined (e.g., accessing input$before_tmax when switching from "Positional BLQ imputation" to "Tmax based imputation"). This could cause the reactive to fail or produce unexpected NULL values. Consider adding req() calls or NULL checks for each input value before using them in the switch statement.

Copilot · 2026-01-02T10:48:49Z

inst/shiny/modules/tab_nca/setup/data_imputation.R

+      # Transform text numbers in the list to numeric
+      rule_list <- lapply(rule_list, function(x) {
+        if (x %in% c("drop", "keep")) {
+          x
+        } else if (grepl("^[0-9]+(\\.[0-9]+)?$", x)) {
+          as.numeric(x)
+        } else {
+          NA
+        }
+      })


When the switch statement doesn't match any case, rule_list is set to NULL (line 86). However, the subsequent lapply call on line 90 will fail with an error when attempting to apply a function over NULL. This occurs when input$select_blq_strategy has an unexpected value. Add a NULL check after the switch statement before processing rule_list.

NEWS.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

inst/shiny/modules/tab_nca/setup/data_imputation.R

R/intervals.R

inst/shiny/modules/tab_nca/setup/data_imputation.R

js3110

For "set value for BLQ" would it not be better to allow a custom input value?
Where does 0.05 come from?

js3110

Would also be good to add some info in the description of what the program considers a BLQ value, so the user is aware

js3110

In DoD it says " allows to introduce a particular BLQ value (i.e LLOQ/2)" I don't see where this is possible?

Shaakon35 · 2026-01-06T14:00:29Z

For "set value for BLQ" would it not be better to allow a custom input value? Where does 0.05 come from?

I have the same comments as Jana.

We would need more flexibility for BLQ handling, the best would be to have these scenarios:

LLOQ / 2 -> for BLQ, instead of 0.5, that should cover cases when multiple analytes have different LLOQ
But for this we would need to map LLOQ, so either we map it as optional and/or we add a text input where we can enter it manually (with no split by analyte :/, it's not optimal but it works for the single analyte studies)

Gero1999 added 4 commits June 23, 2025 08:02

draft: blq imputation options in NCA setup

556e9a7

fix: make work 'Set value for all BLQ'

15b357b

fix: improve invalid input text handling

9884b46

docs: roxygenise

8dab1e1

Gero1999 linked an issue Jun 26, 2025 that may be closed by this pull request

Enhancement: BLQ imputation Options #139

Open

3 tasks

Gero1999 added 8 commits June 26, 2025 19:15

refactor: create blq_selectize helper fun to elude repetition

2a7f54a

refactor: lintr

5b33ac1

docs: roxygenise

61b87d0

fix: default before_tmax should be 0

a643f94

fix: problem in blq_selectize definition for BLQ value

39356c1

Merge remote-tracking branch 'origin/main' into 139-feat/blq-options

f6cf248

data: AVAL < 0.05 was changed to 0 (BLQ)

f6b2c8a

spelling: update wordlist to include BLQ

5e3eee3

Gero1999 marked this pull request as ready for review June 27, 2025 08:48

Gero1999 requested review from js3110 and m-kolomanski June 27, 2025 08:48

js3110 requested changes Jun 27, 2025

View reviewed changes

inst/shiny/modules/tab_nca/setup/data_imputation.R Show resolved Hide resolved

inst/shiny/modules/tab_nca/setup/data_imputation.R Outdated Show resolved Hide resolved

inst/shiny/modules/tab_nca/setup/data_imputation.R Outdated Show resolved Hide resolved

Gero1999 added 2 commits July 1, 2025 10:05

feat: add option for "No BLQ handling"

b745304

data: reset dataset back ADNCA

9c898ec

Gero1999 requested a review from js3110 July 1, 2025 08:47

Shaakon35 added this to aNCA Jul 3, 2025

Shaakon35 removed this from aNCA Jul 3, 2025

Gero1999 marked this pull request as draft July 7, 2025 06:36

merge: solve conflicts in settings / tab_nca

88ccadb

Merge remote-tracking branch 'origin/main' into 139-feat/blq-options # Conflicts: # inst/shiny/modules/tab_nca.R # inst/shiny/modules/tab_nca/setup/settings.R

Gero1999 requested review from Copilot and removed request for Copilot January 2, 2026 09:54

Copilot started reviewing on behalf of Gero1999 January 2, 2026 09:54 View session

refactor: lintr

2e46694

Copilot AI review requested due to automatic review settings January 2, 2026 10:02

Copilot started reviewing on behalf of Gero1999 January 2, 2026 10:02 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

Gero1999 added 4 commits January 2, 2026 11:13

don't require in slope_plots blq_imputation_rule

9940083

english: correct comment grammar

5c49d11

early NULL return use length()==0

b3ba671

docs: just docs change order arg blq_imputation_rule

7e6ec73

Copilot AI review requested due to automatic review settings January 2, 2026 10:42

Copilot started reviewing on behalf of Gero1999 January 2, 2026 10:43 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

Update NEWS.md

8716335

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings January 2, 2026 11:17

Copilot started reviewing on behalf of Gero1999 January 2, 2026 11:18 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

inst/shiny/modules/tab_nca/setup/data_imputation.R Show resolved Hide resolved

R/intervals.R Show resolved Hide resolved

inst/shiny/modules/tab_nca/setup/data_imputation.R Show resolved Hide resolved

Gero1999 added 3 commits January 5, 2026 09:37

start drafting tests for blq_imputation_rule arg

7cab948

finish tests for blq_imputation_rule

14c392a

refactor: lintr

b3edf4f

Gero1999 marked this pull request as ready for review January 5, 2026 10:48

Gero1999 requested review from Copilot and m-kolomanski and removed request for Copilot January 5, 2026 10:48

Copilot started reviewing on behalf of Gero1999 January 5, 2026 10:49 View session

js3110 reviewed Jan 5, 2026

View reviewed changes

	blq_selectize(ns("blq_value"), "Value for BLQ", selected = "0.05"),
	blq_selectize(ns("blq_value"), "Value for BLQ", selected = "0.05")

139 feat/blq options #491

Are you sure you want to change the base?

139 feat/blq options #491

Conversation

Gero1999 commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Description

Definition of Done

How to test

Contributor checklist

Notes to reviewer

Uh oh!

js3110 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gero1999 commented Jun 27, 2025

Uh oh!

js3110 commented Jun 27, 2025

Uh oh!

Gero1999 commented Jul 1, 2025

Uh oh!

Gero1999 commented Jul 7, 2025

Uh oh!

js3110 commented Jul 8, 2025

Uh oh!

billdenney commented Jul 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

js3110 left a comment

Choose a reason for hiding this comment

Uh oh!

js3110 left a comment

Choose a reason for hiding this comment

Uh oh!

js3110 left a comment

Choose a reason for hiding this comment

Uh oh!

Shaakon35 commented Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Gero1999 commented Jun 26, 2025 •

edited

Loading