Skip to content

feat: map lambda.z.corrxy to the CDISC CORRXY term - #1457

Open
billdenney wants to merge 1 commit into
pharmaverse:1102-bug/pknca-dev-compatfrom
billdenney:fix/cdisc-terms-for-new-pknca-params
Open

feat: map lambda.z.corrxy to the CDISC CORRXY term#1457
billdenney wants to merge 1 commit into
pharmaverse:1102-bug/pknca-dev-compatfrom
billdenney:fix/cdisc-terms-for-new-pknca-params

Conversation

@billdenney

Copy link
Copy Markdown
Collaborator

Part of #1102; the CORRXY item on the 0.12.2 checklist in #481.

PKNCA 0.12.1.9000 reports lambda.z.corrxy, the correlation between time and log-concentration for the terminal slope. aNCA had no CDISC entry, so it came through unlabelled and the expected-label test in test-pivot_wider_pknca_results.R failed.

Adds the row to data-raw/metadata_nca_parameters.csv and regenerates data/metadata_nca_parameters.rda. Classified with the other half-life diagnostics (unitless, Standard, Half-life, depends on half.life), matching how r.squared and adj.r.squared are already handled.

@billdenney
billdenney force-pushed the fix/cdisc-terms-for-new-pknca-params branch from 9d87ffe to 66bd170 Compare August 28, 2026 11:44
PKNCA 0.12.1.9000 reports lambda.z.corrxy, the correlation between time
and log-concentration for the terminal slope. aNCA had no CDISC entry for
it, so it came through unlabelled and the expected-label test in
test-pivot_wider_pknca_results.R failed.

Add the row to data-raw/metadata_nca_parameters.csv, regenerate
data/metadata_nca_parameters.rda, and add the parameter to that test's
expected labels. This is the CORRXY item on the 0.12.2 checklist in the
"Things to consider for next PKNCA update" issue.

The parameter is classified with the other half-life diagnostics
(unitless, Standard, Half-life, depends on half.life), matching how
r.squared and adj.r.squared are already handled.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@billdenney
billdenney force-pushed the fix/cdisc-terms-for-new-pknca-params branch from 66bd170 to bc3755f Compare August 28, 2026 12:05
@Gero1999
Gero1999 changed the base branch from main to 1102-bug/pknca-dev-compat August 28, 2026 13:19
@Gero1999

Copy link
Copy Markdown
Collaborator

Thanks for this — the CDISC mapping itself is correct and I verified the rebuilt .rda matches the CSV (all 141 params present, lambda.z.corrxy -> CORRXY with the right PPTEST). Classifying it with the other half-life diagnostics (depends="half.life", unitless, Half-life) is consistent with how r.squared/adj.r.squared are handled.

One concern that needs aNCA-specific context before merge — the test change in test-pivot_wider_pknca_results.R:

aNCA's CI installs PKNCA from CRAN (0.12.1)DESCRIPTION pins PKNCA (>= 0.12.1) and there is no Remotes: entry, so setup-r-dependencies pulls 0.12.1, not the dev build. In 0.12.1, lambda.z.corrxy does not exist (git show v0.12.1:R/half.life.R has zero matches), so pk.nca() never emits a CORRXY column. The expected_labels assertion is an exact ordered expect_equal, so adding CORRXY makes it fail on CI until 0.12.2 is on CRAN:

  • dev PKNCA: CORRXY present -> label needed -> passes ✅
  • CRAN 0.12.1 (what CI runs): CORRXY absent -> extra label -> expect_equal fails ❌

This is the mirror image of the R2/R2ADJ labels already in that vector: those exist in 0.12.1 too, so they're safe today. CORRXY only exists in dev.

The CDISC data change (CSV + .rda) is safe to merge as-is — an unused extra metadata row is harmless on 0.12.1. It's specifically the test assertion that's coupled to the dev PKNCA output.

Options (any is fine, your call):

  1. Make the CORRXY label conditional on it being present in the output (e.g. only assert it when "CORRXY" %in% names(labels)), so the test passes on both PKNCA versions.
  2. Split: land the CSV/.rda now, hold the test assertion until aNCA moves its minimum to the PKNCA release that ships lambda.z.corrxy (add Remotes: humanpred/pknca or bump the floor).
  3. If the intent is that this whole branch targets 0.12.2 and CI will be pointed at dev PKNCA before merge, then it's fine as-is — just flagging so it's a deliberate decision rather than a surprise red check.

Everything else here looks good. Thanks again!

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

Labels

None yet

2 participants