refine templates #593
refine templates #593
Conversation
|
🧪 Code Coverage SummaryDiff against mainResults for commit: c7621d66d8f887d991ffe698233240597c3b50f6 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
|
LGTM, thanks for the updates. |
|
@clarkliming , I reviewed AET02 using study data from the following CENSORED path, no findings in results so far. I noticed that the column labels directly come from the labels of variable being tabulated. The label of columns in an output does not necessarily match exactly with variable label, thus, if the column label is not re-set by default within Chevron, users are expected to re-assign the proper label to the variables, is my understanding of the process correct here? |
you are right. There are usually too many labels to control (and we often need to modify them) and it looks bad to have too many arguments. So we decided to use the labels in the data to control the printing (just like in dmt01, the labels of variables are used to display). So if users want to change the label in table, they just update the label in dataset |
|
@clarkliming , I reviewed CMT02_PT & CMT01A with data from the following CENSORED path, no findings in results. |
| lbl_armvar <- var_labels_for(adam_db$adeg, arm_var) | ||
| lbl_summaryvars <- var_labels_for(adam_db$adeg, summaryvar) | ||
| lbl_splitvar <- var_labels_for(adam_db$adeg, splitvar) | ||
| lbl_param <- var_labels_for(adam_db$adeg, "PARAM") |
There was a problem hiding this comment.
We'd better leave flexibility for users to define lbl_param %||% var_labels_for(adam_db$adeg, "PARAM")
There was a problem hiding this comment.
there is no such argument like lbl_param. And I don't think we should add that argument (will lead to too many arguments)
|
I don't have study data to review EGT03 results, thus, would just review very generally. I think the text below needs some modification to be a little bit more robust. Basically, the input dataset is expected to have one post-baseline record per subject (the selection of records is pre-defined, e.g. post-baseline maximum, post-baseline minimum, first post-baseline observation etc.). For Roche studies, If the output looks at change from baseline to post-baseline maximum, then they should subset AVISIT=POST-BASELINE MAXIMUM; if the outputs looks at change from baseline to post-baseline minimum, they should subset AVISIT=POST-BASELINE MAXIMUM. |
|
LBT14 Help: The following statement is applicable for ADLB created under Roche standards. Different companies actually may have different interpretation of ADaM IG on how to utilize ATOXGR/BTOXGR and how to handle direction in ADLB. If this help page is public, I think it's worthwhile to 1) briefly explain how Roche construct ADLB and the meaning of negative, positive grades 2) mention that the ADLB is following ADaMIG 1.1 (because ADLB following ADaM IG 1.2 changes a lot and this program here would not work) |
|
documentation updated |
|
LBT14: Tested this template with data from the following CENSORED path, no findings in calculated results. However, one comment |
|
the issue can be related to tern. see https://insightsengineering.github.io/tlg-catalog/tables/lab-results/lbt14.html examples. Need to investigate into this. Since chevron provide wrappers around tern, let's move forward (and after tern fix this chevron will work correctly) |
| lbl_row_split <- var_labels_for(adam_db$adae, row_split_var) | ||
| lbl_aedecod <- var_labels_for(adam_db$adae, "AEDECOD") | ||
| lbl_overall <- render_safe(lbl_overall) | ||
| lyt <- aet02_lyt( | ||
| lyt <- occurrence_lyt( |
There was a problem hiding this comment.
just curious for a design purpose: aet02 uses occurence_lyt implemented in cmt01a and in aet02 <- chevron_t() we dont want to expose lyt <- occurence_lyt
There was a problem hiding this comment.
it is defined in main function and no need to expose this function right? the layout thing is internal only
close #592
close #587
provide insights to #588