-
Notifications
You must be signed in to change notification settings - Fork 4
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
Feature Request: derive_param_computed()
wrapper for waist/hip or waist/height ratio
#31
Comments
@pharmaverse/admiralmetabolic thoughts? |
We keep it simple, just 100*WSTCIR /HIPCIR |
Why multiplied by 100? Shouldn't a ratio be 1 rather than 100 if e.g. Waist and Hip circumference are both 100 cm? |
we report this parameter in % |
Interesting. For me the term 'ratio' implied it is rather a unitless parameter (fraction of 1). |
I just looked it up in our (Novo) metadata system, and it has the possibility of being represented as either ratio or percentage. But I would argue that this is a mistake in our system, and the only possible representation should be a ratio. From my understanding, even though ratios can be represented at percentages, it does not make it a percentage. Imagine for instance a WHR above 1 (which is possible), providing percentages can appear really strange. So, I would vote for us to use the ratio and not provide any functionality for percentages. |
Sorry for the confusion. It should be simple ratio WSTCIR /HIPCIR as Kathrin suggested. |
No need to apologize. :) |
I would suggest 3 functions:
+ supplemental Actually, I already have a draft implementation and would be more than happy to share. Looking forward to contributing to @pharmaverse/admiralmetabolic! |
This is a nice idea for setup - I am wondering though if the ratio function would be better in {admiral}? @bms63 @bundfussr what do you think? |
If it can be used by other TAs or general ADaMS then yes we should move it to admiral |
Good point. It's indeed more suitable for {admiral}. What would be the workflow in this kind of case? We propose it to {admiral} by creating an issue/PR there and wait till it becomes available? Or as an alternative we could make it internal here for the moment ( |
Yes - create issue and do a PR. Be best if you work on it or someone from your team. We would merge it to main - and it can easily become available with Remotes or staged.dependencies. There is an admiral released planned for January 2025 that could include this...but we can do a mini-release of admiral if things don't line up perfectly. |
If the ratio function are implemented, it should be in {admiral} because ratios are not metabolic specific. However, I wouldn't implement this setup. We have something similar in admiral but I would consider it as historic. I see no benefit in implementing these functions. With these function we could for example derive the waist-hip ratio by
instead of
The first call doesn't look simpler, shorter, or clearer to me. I think the second call is even clearer than the first one because it is obvious how the ratio derived. Although the new functions are mainly wrapper functions they need to be implemented, documented, and tested, which is some work. And the users and reviewers would need to learn more functions. I would suggest not implementing new |
Thanks @bundfussr for your feedback. I don't think the point of creating wrapper functions is only to simplify the call. It's more about specializing them for a certain purpose. It’s easy for you, with your vast experience with {admiral}, to understand which function is best and how exactly to use it for specific purposes, but not for a user who installed the package five minutes ago. If they need parameters for a ratio, the first thing they’ll do is enter specific keywords in the search bar, and quickly find what they need. In fact, the Besides, specialized wrapper functions may have additional checks for the input arguments or data in order to make it more fool-proof and/or provide the user with some useful info regarding the correctness of their input data. It reminded me of my yesterday’s struggle with the |
Thanks for the feedback @yurovska on admiral
Is it possible for you to reconstruct this simple example so we can include it in our documentation. |
@bms63 Off-topic here but ok, here it is: library(tibble)
ex <- tribble(
~STUDYID, ~USUBJID, ~EXDOSE, ~EXSTDTM,
"STUDY001", "SUBJ001", 2, "2024-01-01T08:00",
"STUDY001", "SUBJ001", 4, "2024-01-02T08:00",
"STUDY001", "SUBJ002", 2, "2024-01-01T08:30",
"STUDY001", "SUBJ002", 4, "2024-01-02T08:30",
"STUDY001", "SUBJ002", 2, "2024-01-03T08:30", # Down-titration
"STUDY001", "SUBJ002", 2, "2024-01-04T08:30",
"STUDY001", "SUBJ003", 2, "2024-01-01T09:00",
"STUDY001", "SUBJ003", 4, "2024-01-03T09:00",
"STUDY001", "SUBJ003", 4, "2024-01-04T09:00",
"STUDY001", "SUBJ003", 8, "2024-01-05T09:00",
"STUDY001", "SUBJ003", 4, "2024-01-06T09:00", # Down-titration
"STUDY001", "SUBJ004", 2, "2024-01-01T10:00",
"STUDY001", "SUBJ004", 4, "2024-01-02T10:00",
"STUDY001", "SUBJ004", 4, "2024-01-03T10:00"
)
ex_flag <- ex %>%
# Flag each EXDOSE which is lower than previous per patient
derive_var_joined_exist_flag(
dataset_add = ex,
by_vars = exprs(STUDYID, USUBJID),
order = exprs(EXSTDTM),
new_var = DOSE_REDUCED,
tmp_obs_nr_var = tmp_dose_nr,
join_vars = exprs(EXDOSE),
join_type = "before",
first_cond_lower = NULL,
first_cond_upper = NULL,
filter_add = NULL,
filter_join = (
tmp_dose_nr == tmp_dose_nr.join + 1 # Look only at adjacent doses (i.e. next to each other)
& EXDOSE > 0 & EXDOSE.join > 0 # Both doses are valid
& EXDOSE < EXDOSE.join # Dose is lower than previous
)
) data ex_flag (drop = prev_dose);
set ex;
by USUBJID EXSTDTM;
if first.USUBJID then call missing(prev_dose);
else prev_dose = lag(EXDOSE);
if 0 < EXDOSE < prev_dose < 0 then DOSE_REDUCED = "Y";
run; |
Thanks @yurovska - I made it an issue in admiral. |
I think we should discuss this issue during our meeting today. To summarize, I think we need to consider the following:
|
Summary of discussion and decision at standup:
To that end, at your earliest convenience @yurovska please update #33 taking note of the above. We leave to your discretion (and the eventual reviewers'):
|
|
Feature Idea
As suggested by @starosto, waist/hip ratio and waist/height ratio are common calculations done at ADaM level. These could be easily added as ADVS params using
admiral::derive_param_computed()
. But should we make a wrapper function for it, similar toderive_param_bmi()
orderive_param_bsa()
If we decide to implement, should we have one or two functions?
Relevant Input
See derive_param_bsa examples.
Relevant Output
See derive_param_bsa examples.
Reproducible Example/Pseudo Code
NA
The text was updated successfully, but these errors were encountered: