Skip to content

fix: rename PK summary exclusion variables to CDISC names (#1394) - #1418

Open
wangzhengdna-lang wants to merge 1 commit into
pharmaverse:mainfrom
wangzhengdna-lang:1394-fix/pksumx-cdisc-rename
Open

fix: rename PK summary exclusion variables to CDISC names (#1394)#1418
wangzhengdna-lang wants to merge 1 commit into
pharmaverse:mainfrom
wangzhengdna-lang:1394-fix/pksumx-cdisc-rename

Conversation

@wangzhengdna-lang

Copy link
Copy Markdown

Issue

Closes #1394

Description

Renames the PK summary exclusion variables to CDISC-compliant names across code, tests, and variable metadata. Exclusion semantics are unchanged ("Y"/1 = excluded, empty/NA = included).

Before After
PKSUM1F PKSUMXF
PKSUM1FN PKSUMXFN
PKSUM1RS PKSUMXRS

Additional details:

  • Internal helper .derive_pksum1rs() renamed to .derive_pksumxrs() for consistency
  • R/zzz.R globalVariables updated, alphabetical order preserved
  • data-raw/metadata_nca_variables.csv + regenerated data/metadata_nca_variables.rda
  • man/add_exclusion_reasons.Rd regenerated via devtools::document()
  • New names added to inst/WORDLIST (digit-free tokens are spell-checked, unlike the old ones)

Metadata deduplication (deviation from a plain rename, flagging for review): the CSV already contained CDISC-named PKSUMXF/PKSUMXFN rows (added in #1306: "PK Summary Exclusion Flag"/"(N)", role Topic). The legacy PKSUM1F/PKSUM1FN rows were therefore removed instead of renamed, keeping the pre-existing CDISC rows. PKSUM1RS had no CDISC counterpart, so it was renamed to PKSUMXRS with label/role aligned to the same family ("PK Summary Exclusion Reason", Topic).

Definition of Done

  • No PKSUM1F / PKSUM1FN / PKSUM1RS remain in the codebase
  • devtools::document() run to regenerate man/ and NAMESPACE
  • NEWS.md updated with the change (and the PR number)
  • Package version incremented
  • lintr and all unit tests pass

How to test

Rename + metadata change; all behavior covered by existing unit tests (updated expectations): test-export_cdisc.R (185 PASS), test-PKNCA.R (126 PASS), test-tlg_module.R (21 PASS). Full suite: 2005 PASS / 0 FAIL, lint 0 violations, spellcheck clean.

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)
  • If any .scss change was done, run data-raw/compile_css.R
  • If a package dependency was added/changed, run data-raw/test_suggests_hidden.R

Notes to reviewer

The metadata deduplication described above is the only judgment call beyond a mechanical rename — the alternative (renaming the legacy rows) would have produced duplicate PKSUMXF/PKSUMXFN entries with conflicting labels/roles.

…e#1394)

Rename PKSUM1F/PKSUM1FN/PKSUM1RS to PKSUMXF/PKSUMXFN/PKSUMXRS across
code, tests, and variable metadata; exclusion semantics unchanged
("Y"/1 = excluded, empty/NA = included).

- Helper .derive_pksum1rs() renamed to .derive_pksumxrs()
- metadata_nca_variables.csv: removed legacy PKSUM1F/PKSUM1FN rows in
  favor of the pre-existing CDISC-named PKSUMXF/PKSUMXFN rows (added in
  pharmaverse#1306); PKSUM1RS renamed to PKSUMXRS with label/role aligned to the
  CDISC family ("PK Summary Exclusion Reason", Topic)
- New names added to inst/WORDLIST (digit-free tokens are spell-checked)
- Regenerated metadata_nca_variables.rda and add_exclusion_reasons.Rd
- Bump version to 0.1.0.9189
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: rename PK summary exclusion flags to CDISC-compliant names (PKSUM1F → PKSUMXF)

1 participant