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.

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

18 EXT01 #41

Merged
merged 17 commits into from
Sep 23, 2021
Merged

18 EXT01 #41

merged 17 commits into from
Sep 23, 2021

Conversation

BFalquet
Copy link
Contributor

close insightsengineering/chevron-tasks#14

Create ext01 table which could serve as template for tables that require summary statistics and binning of continuous variables in the same table. End result is slightly different from the one presented in the TLG catalog for the supplementary table.
Thank you for the review .

if(prune_0) tbl <- tbl %>% prune_table()
if (prune_0)
tbl <- prune_table(tbl)
else
Copy link
Contributor

Choose a reason for hiding this comment

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

else can be removed

R/ext01.R Outdated
@@ -0,0 +1,227 @@
#' Exposure Summary Table
#'
#'The EXT01 table provides an overview of the of the exposure of the patients in terms of Total dose administered or
Copy link
Contributor

Choose a reason for hiding this comment

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

please add space

R/ext01.R Outdated
)) {

# provide a clearer error message in the case of missing variable
missing_var = setdiff(summaryvars, colnames(adex))
Copy link
Contributor

Choose a reason for hiding this comment

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

please us <- and not =

Copy link
Contributor

Choose a reason for hiding this comment

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

please create a utility function error_if_missing_vars

R/ext01.R Outdated

# provide a clearer error message in the case of missing variable
missing_var = setdiff(summaryvars, colnames(adex))
if(length(missing_var) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please add space before (, use the lintr plugin to lint your package

R/ext01.R Outdated
if(prune_0) tbl <- tbl %>% prune_table()

#TDOD: sort
tbl_sorted <- tbl
Copy link
Contributor

Choose a reason for hiding this comment

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

please sort?

R/ext01.R Outdated
#' @param group `(nested list)` providing for each parameter value that should be analyzed in a categorical way: the
#' name of the parameter `(string)`, a series of breakpoints `(vector)` where the first breakpoints is typically `-Inf`
#' and the last `Inf`, and a series of name which will describe each category `(vector)`.
#'@param paramvar `(vector)` providing the name of the parameters whose statistical summary should be presented. To
Copy link
Contributor

Choose a reason for hiding this comment

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

please add space here

R/ext01.R Outdated
c("<5000", "5000-7000", "7000-9000", ">9000")
)
),
paramvar = c(""),
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. when can you change the paramvar? isn't it always PARAM? i.e. remove the argument
  2. if argument needed, then empty string is probably not allowed.
  3. does this go into the study object and should be called paramvar_adex?.

R/ext01.R Outdated
)
}

adex$AVAL_gp <- NA #NA
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we deriving this and why is it not in AVALC?

R/ext01.R Outdated
if(paramvar != "ALL") {

adex <- adex %>%
mutate(AVAL = ifelse(PARAM %in% paramvar, AVAL, NA))
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this not already in the adam dataset?

@BFalquet BFalquet merged commit 1d90427 into main Sep 23, 2021
@BFalquet BFalquet deleted the 18_ext01@main branch September 23, 2021 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants