-
Notifications
You must be signed in to change notification settings - Fork 29
PPANMETH in pk.nca output #469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Can you please change this from optional to always present? It can be ignored if the user does not want it, but it will keep the code simpler and not be a burden to the user to always create it rather than it being optional. Please also merge in the current |
R/pk.calc.all.R
Outdated
| call_args$time <- call_args$time[!exclude_tf] | ||
| } | ||
| } | ||
| # For half-life related parameters, indicate if there was any manual inclusion / exclusion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather that the method be applied within the calculation function rather than at the intervals level. That way, if we add more PPANMETH values in the future (e.g. first vs last cmax) it will apply directly.
Perhaps the best/easiest way is to make it work the same way that exclusions work now with an attribute on the returned object.
I'd also be open to having a ppanmeth = FALSE flag set for functions which would have them return a data.frame with PPANMETH, if TRUE. (Though I generally prefer consistent return value classes rather than return value classes that depend on input-- for ease of general use.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, this logical path seems better. I have some difficulties imaging these implementations. Lmk if you can help me, otherwise I will just try to address them naively:
attributes
- The regular user won't check attr and will see output differences between
pk.calc (attr)&pk.nca (col)that cannot be replicated - Most of the expect_equal tests associated with
pk.ncaresults and derived obj don't ignore attributes.
ppanmeth = FALSE
- Imputations should be indicated in PPANMETH, that also helps distinguishing the results from same intervals with different imputations. I guess it should be once per interval. If I follow the attr strategy I can see using vectors, but with the pk.calc(ppanmeth = FALSE) strategy then, how do I elude repetition?
|
Some aspects I noticed that I noticed in the original code that I am not sure if they are intentional:
|
This PR allows in
pk.ncafunction the addition of an optionalPPANMETHcolumn, which records the analysis method specifications for each calculated parameter. This helps users understand how each result was derived, such as the AUC calculation method, half-life selection, and any imputation performed. The change was documented and tested.Closes #457