Closes #34 function for growth parameters for height/length #45
Conversation
|
zdz2101
requested review from
rossfarrugia,
kabis-ops,
Fanny-Gautier and
Lina2689
There was a problem hiding this comment.
@zdz2101 i focused my review on the documentation and trust @kabis-ops and others to check the function code. and we can add more unit tests later as per the plan for the other function.
| @@ -93,14 +92,13 @@ | |||
| #' advs <- dm_peds %>% | |||
| #' select(USUBJID, BRTHDTC, SEX) %>% | |||
| #' right_join(., vs_peds, by = "USUBJID") %>% | |||
| #' filter(USUBJID != "PEDS-1010") %>% | |||
| #' mutate( | |||
| #' VSDT = ymd(VSDTC), | |||
| #' BRTHDT = ymd(BRTHDTC) | |||
| #' ) %>% | |||
| #' derive_vars_duration( | |||
There was a problem hiding this comment.
trunc_out = FALSE can be removed as its default anyway for this function - to simplify the example
| @@ -93,14 +92,13 @@ | |||
| #' advs <- dm_peds %>% | |||
There was a problem hiding this comment.
can we add comments to this example please? as its quite a lot for users to follow with so much pre-processing before we even get to the example function call
R/derive_params_growth_height.R
Outdated
| #' The CDC/WHO growth chart metadata datasets are available in the package and will | ||
| #' require small modifications. | ||
| #' * `HEIGHT` - Height/Length | ||
| #' * `HEIGHTU` - Height Unit |
There was a problem hiding this comment.
HEIGHTU - Height/Length Unit instead of only Height unit. I think that the name is confusing when length is selected
R/derive_params_growth_height.R
Outdated
| #' parameter = VSTESTCD == "WEIGHT", | ||
| #' analysis_var = VSSTRESN, | ||
| #' set_values_to_sds = exprs( | ||
| #' PARAMCD = "WTASDS", |
There was a problem hiding this comment.
Should be PARAMCD = "WTHSDS"?
R/derive_params_growth_height.R
Outdated
| #' ) | ||
| #' | ||
| #' | ||
| #' derive_params_growth_height( |
There was a problem hiding this comment.
Where is patient 01-701-1033 gone ? While running this code I don't have any records created for patient 01-701-1033. Could you please clarify the reason ? thanks
There was a problem hiding this comment.
patient should be in there, at least showing on my end
R/derive_params_growth_height.R
Outdated
| #' right_join(., heights, by = c("USUBJID", "VSDTC")) | ||
| #' | ||
| #' advs_under2 <- advs %>% | ||
| #' filter(AGECUR_M <= 2) |
There was a problem hiding this comment.
Why do you select patients under 2 months only ? should it not be 24 months to consider 2 years ?
There was a problem hiding this comment.
my mistake this was fixed
R/derive_params_growth_height.R
Outdated
| #' filter(AGECUR_M <= 2) | ||
| #' | ||
| #' advs_over2 <- advs %>% | ||
| #' filter(AGECUR_M > 2) |
There was a problem hiding this comment.
Why do you select patients over 2 months only ? should it not be 24 months to consider 2 years ?
| #' Derive Anthropometric indicators (Z-Scores/Percentiles-for-Age) based on Standard Growth Charts | ||
| #' for Weight/BMI/Head Circumference by Height/Length | ||
| #' | ||
| #' @param dataset Input dataset |
There was a problem hiding this comment.
In the template we selected the records from the metadata where MEASURE ="LENGTH" for patients <730.5 days, and MEASURE="HEIGHT" for patients >=730.5 days. As Zelos mentioned, it will require additional variables to merge the right data depending on the age.
We also added message in the ADVS peds template for the same message("To derive height/length parameters, below function needs to call separately for Height and Length based on the input data and current age of the patient, as it depends on your CRF guidelines.").
I think it is easier to split it because if the user has only HEIGHT then there is only one call, similarly for LENGTH. But if the user has both LENGTH and HEIGHT in the data, it will complexify the merge. I think the user has more flexibility while splitting the derivation.
| #' * `M` - Median | ||
| #' * `S` - Coefficient of variation | ||
| #' | ||
| #' @param parameter Anthropometric measurement parameter to calculate z-score or percentile |
There was a problem hiding this comment.
Shall we include a reminder that the expected unit of weight is kg?
| #' | ||
| #' The dataset can be derived from WHO or user-defined datasets. | ||
| #' The WHO growth chart metadata datasets are available in the package and will | ||
| #' require small modifications. |
There was a problem hiding this comment.
The original WHO metadata table has height/length increment of 0.5, but the metadata in the admiralpeds packages has increment of 0.1. How was it extrapolated? Do we have any documentation on that?
There was a problem hiding this comment.
https://www.who.int/tools/child-growth-standards/standards/weight-for-length-height
scroll to the bottom on the expanded tables section you will find the increments were by 0.1
| #' `HEIGHT_LENGTH`, `HEIGHT_LENGTHU`, `SEX`, `L`, `M`, `S` | ||
| #' | ||
| #' The dataset can be derived from WHO or user-defined datasets. | ||
| #' The WHO growth chart metadata datasets are available in the package and will |
There was a problem hiding this comment.
Shall we list out the names of the metadata and clarify that "who_wt_for_lgth_boys" and "who_wt_for_lgth_girls" are for subjects with age <730.5 days, and "who_wt_for_ht_boys" and "who_wt_for_ht_girls" are for those with age>=730.5 days? By doing this, the user knows the metadata he/she can use, and also knows to apply different metadata based on subjects' age.
There was a problem hiding this comment.
This is a good idea, but I think we'll need to draw up a bigger vignette/article for metadata creation/preprocessing anyway
There was a problem hiding this comment.
hopefully the template and vignette can cover this more so user gets the full context
| #' PARAMCD = "WTHPCTL", | ||
| #' PARAM = "Weight-for-height percentile" | ||
| #' ) | ||
| #' ) |
There was a problem hiding this comment.
Shall we include codes to bind rows of advs_under2 and advs_over2?
There was a problem hiding this comment.
Makes sense just for the sake of completing the example
R/derive_params_growth_height.R
Outdated
| #' new_var_unit = AAGECURU, | ||
| #' start_date = BRTHDT, | ||
| #' end_date = VSDT, | ||
| #' out_unit = "months" |
There was a problem hiding this comment.
In the ad_advs.R template (PR #32 ), the unit of age is days (with 730.5 days as cutoff). Shall we be consistent in the example here?
There was a problem hiding this comment.
Yes I will fix this
|
Hi @zdz2101 , I've added a few comments on the PR (on the |
R/derive_params_growth_height.R
Outdated
| #' PARAM = "Weight-for-height z-score" | ||
| #' ), | ||
| #' set_values_to_pctl = exprs( | ||
| #' PARAMCD = "WTHPCTL", |
There was a problem hiding this comment.
@zdz2101 please modify the PARAMCD to WGTHSDS/WGTHPCTL to be consistent with the ADVS template. Same changes are required for derive_params_by_age() function as well.
| @@ -0,0 +1,41 @@ | |||
| # derive_params_growth_heightlength ---- | |||
|
|
|||
| ## Test 1: derive_params_growth_heightlength works ---- | |||
There was a problem hiding this comment.
Some outliers data to be added
Thank you for your Pull Request! We have developed this task checklist from the Development Process Guide to help with the final steps of the process. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the admiral codebase remains robust and consistent.
Please check off each taskbox as an acknowledgment that you completed the task or check off that it is not relevant to your Pull Request. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the
mainbranch until you have checked off each task.styler::style_file()to style R and Rmd filesdevtools::document()so all.Rdfiles in themanfolder and theNAMESPACEfile in the project root are updated appropriatelyNEWS.mdunder the header# admiralpeds (development version)if the changes pertain to a user-facing function (i.e. it has an@exporttag) or documentation aimed at users (rather than developers)pkgdown::build_site()and check that all affected examples are displayed correctly and that all new functions occur on the Reference page.lintr::lint_package()R CMD checklocally and address all errors and warnings -devtools::check()