Summary
The PK helper functions added in #289 (R/pk_helper.R) have a few defects and gaps that should be addressed.
1. imputation_rules() — postdose argument is dead code
The two 1/3 rule branches (!postdose and postdose) are byte-for-byte identical, so the postdose flag has no effect on the output. The argument adds a required parameter and implies behaviour that does not exist.
Post-dose vs pre-dose handling is a data step (e.g. substituting post-dose LTRs with half the LLOQ before summarizing), not a display decision. That logic belongs upstream in the caller/template, before statistics are computed. imputation_rules() should only decide how an already-computed statistic is displayed given the BLQ ratio.
Proposed fix: remove postdose from the signature and collapse the two identical branches into one.
2. geom_mean() returns NaN for all-NA input
geom_mean(c(NA, NA)) returns NaN rather than NA_real_. The any(x <= 0, na.rm = na.rm) guard can also propagate NA into if() when na.rm = FALSE.
Proposed fix: drop NAs up front (when na.rm = TRUE), return NA_real_ for the empty/all-non-positive cases.
3. Formatters are not vectorized and bake in domain logic
fmt_3sig() and fmt_pct() accept only length-1 input and return the string "NE" for NA. "Not estimable" is analysis semantics, not formatting — mixing it into a generic formatter is surprising and makes the functions hard to reuse.
Proposed fix: vectorize both formatters and return NA_character_ for NA/non-finite input. The "NE"/"ND" substitution stays in imputation_rules() where it belongs.
4. Missing documentation
cv(), geom_cv(), and fmt_pct() lack @description and lifecycle badges that the other helpers have.
5. No test coverage
R/pk_helper.R shipped without a tests/testthat/test-pk_helper.R.
Scope
All of the above will be fixed in one PR. The public API change is the removal of postdose from imputation_rules(); the dependent tlg.templates PKCT01 template call will be updated separately.
Summary
The PK helper functions added in #289 (
R/pk_helper.R) have a few defects and gaps that should be addressed.1.
imputation_rules()—postdoseargument is dead codeThe two
1/3rule branches (!postdoseandpostdose) are byte-for-byte identical, so thepostdoseflag has no effect on the output. The argument adds a required parameter and implies behaviour that does not exist.Post-dose vs pre-dose handling is a data step (e.g. substituting post-dose LTRs with half the LLOQ before summarizing), not a display decision. That logic belongs upstream in the caller/template, before statistics are computed.
imputation_rules()should only decide how an already-computed statistic is displayed given the BLQ ratio.Proposed fix: remove
postdosefrom the signature and collapse the two identical branches into one.2.
geom_mean()returnsNaNfor all-NAinputgeom_mean(c(NA, NA))returnsNaNrather thanNA_real_. Theany(x <= 0, na.rm = na.rm)guard can also propagateNAintoif()whenna.rm = FALSE.Proposed fix: drop
NAs up front (whenna.rm = TRUE), returnNA_real_for the empty/all-non-positive cases.3. Formatters are not vectorized and bake in domain logic
fmt_3sig()andfmt_pct()accept only length-1 input and return the string"NE"forNA. "Not estimable" is analysis semantics, not formatting — mixing it into a generic formatter is surprising and makes the functions hard to reuse.Proposed fix: vectorize both formatters and return
NA_character_forNA/non-finite input. The"NE"/"ND"substitution stays inimputation_rules()where it belongs.4. Missing documentation
cv(),geom_cv(), andfmt_pct()lack@descriptionand lifecycle badges that the other helpers have.5. No test coverage
R/pk_helper.Rshipped without atests/testthat/test-pk_helper.R.Scope
All of the above will be fixed in one PR. The public API change is the removal of
postdosefromimputation_rules(); the dependenttlg.templatesPKCT01 template call will be updated separately.