Skip to content
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.

Sign up for GitHub

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

Closes #34 function for growth parameters for height/length #45

Merged
merged 22 commits into from
Jun 17, 2024
Merged

Closes #34 function for growth parameters for height/length #45

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5ba5655
rough draft up
zdz2101 May 30, 2024
70271b3
styler lintr spelling
zdz2101 May 30, 2024
e7f97e7
fix docs
zdz2101 May 30, 2024
879fe9f
styler
zdz2101 May 30, 2024
e5a231b
lintr
zdz2101 May 30, 2024
357c726
add example
zdz2101 May 31, 2024
719fd83
rename metadata required height/length
zdz2101 Jun 3, 2024
7e3af6e
apply changes to test
zdz2101 Jun 3, 2024
af7d60c
missed a piece
zdz2101 Jun 3, 2024
52b2885
lintr
zdz2101 Jun 3, 2024
d56dab4
adopt most feedback
zdz2101 Jun 5, 2024
5d11c2f
adopt feedback
zdz2101 Jun 6, 2024
9bc420f
lintr
zdz2101 Jun 6, 2024
feba775
typo
zdz2101 Jun 7, 2024
0918f3d
Merge branch 'main' into 34_new_func_lengthheight
zdz2101 Jun 12, 2024
c9015a8
adopt feedback
zdz2101 Jun 12, 2024
d550a0d
lintr
zdz2101 Jun 12, 2024
14270f6
lintr
zdz2101 Jun 12, 2024
4fa8ceb
WG for WT
zdz2101 Jun 13, 2024
6fd61fc
Merge branch 'main' into 34_new_func_lengthheight
rossfarrugia Jun 17, 2024
617f781
Merge branch 'main' into 34_new_func_lengthheight
rossfarrugia Jun 17, 2024
b3bf788
spelling and man files
rossfarrugia Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 21 additions & 23 deletions R/derive_params_growth_height.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#' Derive Anthropometric indicators (Z-Scores/Percentiles-for-Age) based on Standard Growth Charts
#' Derive Anthropometric indicators (Z-Scores/Percentiles-for-Height) based on Standard Growth Charts
#'
#' Derive Anthropometric indicators (Z-Scores/Percentiles-for-Age) based on Standard Growth Charts
#' for Weight/BMI/Head Circumference by Height/Length
#' Derive Anthropometric indicators (Z-Scores/Percentiles-for-Height) based on Standard Growth Charts
#' for Weight by Height/Length
#'
#' @param dataset Input dataset
#'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing parameters from the issue:
age,
age_unit,
height_age,
These are all needed as we need to know at which age to assume height instead of body length but they're optional as: If only ever length or height is used then leave this NULL and just feed in only the corresponding by length or height metadata (instead of the combined version which has both)

I agree measure argument in the issue not needed - as we can add the height temp var to the input dataset

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so that you use the right metadata from WHO depending on patient age and which way they were likely measuring (height or body length). it'd be good in the examples to use height_age = 730.5 days i.e. 2 years (even as default from what David explained to us?). See https://github.com/pharmaverse/admiralpeds/blob/35_advs_vignette/vignettes/advs.Rmd from line 229 for further explanation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rossfarrugia I think that's why in the example I actually split the dataset into over 2 and under 2 and just ran the function twice, otherwise you would need to create some sort of additional joining variable on both sides, dataset and metadata, which involves additional pre-processing to both datasets, while adding complexity to the function too, the "modularity" of running it twice felt more intuitive to me

I'm open to this adoption with additional arguments, but I wonder what other programmers would think

Copy link
Collaborator

@rossfarrugia rossfarrugia Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zdz2101 i'm open to your approach - it does give the user complete control still. we'll just need to well comment this to explain our approach in your roxygen2 function documentation example (e.g. at the end you should add a comment to explain that the 2 resulting dataframes would need to be set back together to get the complete ADVS for this parameter) and also we'll need to explain well in our template comments and our vignette.

@Fanny-Gautier @Lina2689 what do you think? as the template authors i would trust your advice here as you'll have a good read on what makes this least complex for users.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense Fanny, sounds like we have a plan then! and thanks for the other comments here too - looks like me and you picked up similar spots.

#' The variables specified in `sex`, `age`, `age_unit`, `parameter`, `analysis_var`
#' The variables specified in `sex`, `height`, `height_unit`, `parameter`, `analysis_var`
#' are expected to be in the dataset.
#'
#' @param sex Sex
Expand Down Expand Up @@ -46,8 +46,7 @@
#'
#' e.g. `parameter = VSTESTCD == "WEIGHT"`.
#'
#' There is CDC/WHO metadata available for Height, Weight, BMI, and Head Circumference available
#' in the `admiralpeds` package.
#' There is WHO metadata available for Weight available in the `admiralpeds` package.
#'
#' @param analysis_var Variable containing anthropometric measurement
#'
Expand All @@ -57,7 +56,7 @@
#'
#' The specified variables are set to the specified values for the new
#' observations. For example,
#' `set_values_to_sds(exprs(PARAMCD = “BMIASDS”, PARAM = “BMI-for-height z-score”))`
#' `set_values_to_sds(exprs(PARAMCD = “WTASDS”, PARAM = “Weight-for-height z-score”))`
#' defines the parameter code and parameter.
#'
#' *Permitted Values*: List of variable-value pairs
Expand All @@ -68,7 +67,7 @@
#'
#' The specified variables are set to the specified values for the new
#' observations. For example,
#' `set_values_to_pctl(exprs(PARAMCD = “BMIAPCTL”, PARAM = “BMI-for-height percentile”))`
#' `set_values_to_pctl(exprs(PARAMCD = “WTAPCTL”, PARAM = “Weight-for-height percentile”))`
#' defines the parameter code and parameter.
#'
#' *Permitted Values*: List of variable-value pair
Expand All @@ -93,14 +92,13 @@
#' advs <- dm_peds %>%
#' select(USUBJID, BRTHDTC, SEX) %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

#' right_join(., vs_peds, by = "USUBJID") %>%
#' filter(USUBJID != "PEDS-1010") %>%
#' mutate(
#' VSDT = ymd(VSDTC),
#' BRTHDT = ymd(BRTHDTC)
#' ) %>%
#' derive_vars_duration(
#' new_var = AGECUR_M,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trunc_out = FALSE can be removed as its default anyway for this function - to simplify the example

#' new_var_unit = CURU_M,
#' new_var = AAGECUR,
#' new_var_unit = AAGECURU,
#' start_date = BRTHDT,
#' end_date = VSDT,
#' out_unit = "months",
Expand All @@ -111,18 +109,18 @@
#' filter(VSTESTCD == "HEIGHT") %>%
#' select(USUBJID, VSSTRESN, VSSTRESU, VSDTC) %>%
#' rename(
#' HEIGHT = VSSTRESN,
#' HEIGHTU = VSSTRESU
#' HGTTMP = VSSTRESN,
#' HGTTMPU = VSSTRESU
#' )
#'
#' advs <- advs %>%
#' right_join(., heights, by = c("USUBJID", "VSDTC"))
#'
#' advs_under2 <- advs %>%
#' filter(AGECUR_M <= 2)
#' filter(AAGECUR <= 24)
#'
#' advs_over2 <- advs %>%
#' filter(AGECUR_M > 2)
#' filter(AAGECUR > 24)
#'
#' who_under2 <- bind_rows(
#' (admiralpeds::who_wt_for_lgth_boys %>%
Expand Down Expand Up @@ -166,36 +164,36 @@
#' derive_params_growth_height(
#' advs_under2,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

patient should be in there, at least showing on my end

#' sex = SEX,
#' height = HEIGHT,
#' height_unit = HEIGHTU,
#' height = HGTTMP,
#' height_unit = HGTTMPU,
#' meta_criteria = who_under2,
#' parameter = VSTESTCD == "WEIGHT",
#' analysis_var = VSSTRESN,
#' set_values_to_sds = exprs(
#' PARAMCD = "WTASDS",
#' PARAM = "Weight-for-age z-score"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be PARAMCD = "WTHSDS"?

#' PARAM = "Weight-for-height z-score"
#' ),
#' set_values_to_pctl = exprs(
#' PARAMCD = "WTAPCTL",
#' PARAM = "Weight-for-age percentile"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be PARAMCD = "WTHPCTL"?

#' PARAM = "Weight-for-height percentile"
#' )
#' )
#'
#' derive_params_growth_height(
#' advs_over2,
#' sex = SEX,
#' height = HEIGHT,
#' height_unit = HEIGHTU,
#' height = HGTTMP,
#' height_unit = HGTTMPU,
#' meta_criteria = who_over2,
#' parameter = VSTESTCD == "WEIGHT",
#' analysis_var = VSSTRESN,
#' set_values_to_sds = exprs(
#' PARAMCD = "WTASDS",
#' PARAM = "Weight-for-age z-score"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be PARAMCD = "WTHSDS"?

#' PARAM = "Weight-for-height z-score"
#' ),
#' set_values_to_pctl = exprs(
#' PARAMCD = "WTAPCTL",
#' PARAM = "Weight-for-age percentile"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be PARAMCD = "WTHPCTL"?

#' PARAM = "Weight-for-height percentile"
#' )
#' )
derive_params_growth_height <- function(dataset,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we include codes to bind rows of advs_under2 and advs_over2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense just for the sake of completing the example

Expand Down
44 changes: 21 additions & 23 deletions man/derive_params_growth_height.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading