docs: metadata and dead-code cleanups (normalize_by_col title, NEWS, duplicate registrations, setRoute) - #592
Open
billdenney wants to merge 2 commits into
Open
docs: metadata and dead-code cleanups (normalize_by_col title, NEWS, duplicate registrations, setRoute)#592billdenney wants to merge 2 commits into
billdenney wants to merge 2 commits into
Conversation
…duplicate registrations, setRoute)
Cleanups for issue 584:
- Retitle normalize_by_col() roxygen docs as user-facing ("Normalize
PKNCA results by a column of the concentration data"); the function is
exported, so "Internal function" was misleading. Regenerated
man/normalize_by_col.Rd.
- NEWS.md: correct the sparse parameter bullet to the five sparse-derived
parameters actually registered (mrt.sparse.last, not mrt.ivint.last)
and add the missing entry for PKNCA_impute_method_end_conc_drop().
- Fix stale comment in pk.calc.all.R pointing at
"Writing-Parameter-Functions.Rmd" to the current
"vignettes/v80-writing-parameter-functions.Rmd".
- Remove duplicated add.interval.col() registrations for cl.sparse.last,
kel.sparse.last, mrt.sparse.last, vss.sparse.last, and vz.sparse.last
in pk.calc.simple.R. The pairs were verified identical and
get.interval.cols() output confirmed byte-identical (203 entries)
before and after removal.
- Remove dead message_if_default wiring in setRoute.PKNCAdose(): the
message could never fire because default_value is always supplied.
The route silently defaults to extravascular by design (no behavior
change).
- Add a registry test pinning that each sparse-derived parameter is
registered exactly once and the total registry size is stable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Additional cleanup for issue 584: setDuration.PKNCAdose() carried the same dead message_if_default pattern already removed from setRoute.PKNCAdose(). setAttributeColumn() consults message_if_default only when default_value is missing, and default_value=0 is always supplied, so "Assuming instant dosing (duration=0)" could never fire (confirmed empirically: creating a PKNCAdose without duration/rate emits no message). The duration silently defaults to 0 by design; no behavior change. No test referenced the message. Also extend the NEWS dead-wiring bullet to cover both setRoute and setDuration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #584.
Applies the grab-bag:
normalize_by_col()retitled as user-facing; the NEWS sparse bullet corrected to the actual five sparse-derived parameters (mrt.sparse.last, notmrt.ivint.last) and reworded from "sparse AUC parameters"; a NEWS entry added forPKNCA_impute_method_end_conc_drop; the stale source comment now points atvignettes/v80-writing-parameter-functions.Rmd; the duplicatedadd.interval.col()registrations for the five sparse-derived parameters removed (registry provenidentical()before/after, and a new test pins single registration plus total registry size); the unreachablemessage_if_defaultwiring removed fromsetRoutewith a comment that the route defaults silently by design.Verification: 345 tests pass across the touched areas.
Note:
setDurationhas the same deadmessage_if_defaultpattern — left out of scope here.🤖 Generated with Claude Code