Skip to content

Forward-compatibility with the upcoming PKNCA release#1355

Open
billdenney wants to merge 3 commits into
pharmaverse:mainfrom
billdenney:pknca-dev-compat
Open

Forward-compatibility with the upcoming PKNCA release#1355
billdenney wants to merge 3 commits into
pharmaverse:mainfrom
billdenney:pknca-dev-compat

Conversation

@billdenney

Copy link
Copy Markdown
Collaborator

Summary

The development version of PKNCA (0.12.1.9000) breaks aNCA in three places. This makes
aNCA forward-compatible while preserving current behavior on released PKNCA (0.12.1).

1. Half-life inclusion no longer errors

PKNCA dev rejects an interval that has both include_half.life and exclude_half.life
"in use", and treats a column that is not entirely NA (e.g. all-FALSE) as in use.
aNCA initialized exclude_half.life to FALSE, so selecting points to include aborted
pk.nca() with "Cannot both include and exclude half-life points for the same interval".

Fix: initialize exclude_half.life to NA (matching include_half.life, already left
NA until a point is selected). Reads are guarded with %in% TRUE, so NA and FALSE
are treated identically ("not excluded") — no behavior change under released PKNCA.

2. New PKNCA parameters not yet mapped to CDISC (tests skipped on CRAN)

PKNCA dev emits parameters aNCA does not yet map: lambda.z.corrxy (new
pk.calc.half.life() output) appears unlabeled in pivot_wider_pknca_results(), and new
vz.* params such as vz.last create extra Vz rows in export_cdisc(). The two affected
tests are marked skip_on_cran() so an upcoming PKNCA release cannot block aNCA on CRAN,
while the mismatch still surfaces off-CRAN until the mapping is extended.
Follow-up: add the new params to data-raw/metadata_nca_parameters.csv and export_cdisc().

Verification

  • R CMD check against PKNCA dev: OK (0 errors / 0 warnings / 0 notes)
  • Full test suite passes (the two parameter tests skip on CRAN); lintr clean

🤖 Generated with Claude Code

billdenney and others added 2 commits June 13, 2026 21:45
The development version of PKNCA rejects an interval that has both
include_half.life and exclude_half.life "in use", where a column counts
as in use when it is not entirely NA. aNCA initialized exclude_half.life
to FALSE, so as soon as a user selected points to include, PKNCA aborted
with "Cannot both include and exclude half-life points for the same
interval".

Initialize exclude_half.life to NA (matching include_half.life, which is
already left NA until a point is selected). Reads of the column are
guarded with `%in% TRUE` so NA and FALSE behave identically ("not
excluded"), preserving behavior under released PKNCA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The development version of PKNCA emits parameters that aNCA does not yet
map to CDISC: lambda.z.corrxy (a new pk.calc.half.life output) flows into
pivot_wider_pknca_results() unlabeled, and new vz.* parameters such as
vz.last produce extra Vz rows in export_cdisc(). Both make hardcoded test
expectations fail.

Mark the two affected tests skip_on_cran() so an upcoming PKNCA release
cannot block aNCA on CRAN, while the mismatch still surfaces off-CRAN
(e.g. in CI run against PKNCA dev) until the CDISC mapping is extended.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@billdenney

Copy link
Copy Markdown
Collaborator Author

I would like to release a new version of PKNCA soon (within the next ~2 weeks) and I know that an ANCA CRAN failure will hold up the PKNCA release, so can you please address these issues? I am separately working through humanpred/pknca#536 which would move the parameter assignment within PKNCA and then prevent one of these errors in the future.

@Shaakon35

Copy link
Copy Markdown
Collaborator

@Gero1999 it looks good for me but I would need your review to merge this

@Gero1999

Gero1999 commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

@Shaakon35 for testing properly please make sure to install the GitHub PKNCA version. There are some problems with the HL plots rendering correctly due to the new errors introduced by PKNCA. They don't allow simultaneous selection+exclusion.

@billdenney I gave you write access so in the future you can create repo-branches. I will create a new one and rebase yours so we can keep working in there, and afterwards open a new PR

@Shaakon35

Copy link
Copy Markdown
Collaborator

Two items from review:

1. skip_on_cran() is too broad (medium)

The two skip_on_cran() additions skip tests unconditionally on CRAN regardless of which PKNCA version is installed. If a future released PKNCA ships the breaking changes before aNCA adds the parameter mappings, these tests will silently pass on CRAN. A version-conditional skip would be more precise, e.g.:

skip_if(packageVersion("PKNCA") >= "0.12.2", "New PKNCA params not yet mapped")

2. Test fixture not updated (medium)

tests/testthat/setup.R line 148 still initializes exclude_half.life = FALSE in FIXTURE_PKNCA_DATA. Production code now initializes to NA. Tests pass because %in% TRUE treats both identically, but the fixture doesn't reflect the actual production state. Future tests that assume the fixture matches production could have subtle bugs.

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.

3 participants