94 refactor #95

waddella · 2021-11-18T11:34:58Z

Please have a look at aet02.R

linting

…le-subtitle-and-footnotes@main

github-actions · 2021-11-18T11:41:21Z

File	Coverage		Missing
All files	`0%`	❌
R/aet02.R	`0%`	❌	`61-441`
R/aet03.R	`0%`	❌	`49-154`
R/aet04.R	`0%`	❌	`59-168`
R/assertthat.R	`0%`	❌	`3`
R/dmt01.R	`0%`	❌	`54-117`
R/dst01.R	`0%`	❌	`6-559`
R/ext01.R	`0%`	❌	`47-194`
R/gen_args.R	`0%`	❌	`44`
R/lbt01.R	`0%`	❌	`50-121`
R/standard_data_manipulation.R	`0%`	❌	`31-201`
R/utils.R	`0%`	❌	`13-384`

Minimum allowed coverage is 80%

Generated by 🐒 cobertura-action against d2f647e

waddella

@BFalquet & @wangh107 please have a look and let's discuss these changes. I have added some review comments to clarify the changes

waddella · 2021-11-18T11:39:47Z

R/aet02.R

 #'
-#' aet02_1(adsl, adae)
-#' aet02_1(adsl, adae, lbl_overall = "All Patients")
+#' db <- syn_test_data() %>%


the syn_test_data makes the examples easier, we should add the preprocessing that is needed for other examples as well

waddella · 2021-11-18T11:40:15Z

R/aet02.R

-#' aet02_1(adsl, adae)
+#' db <- db %>%
+#'   (std_filter("aet02_1"))() %>%
+#'   (std_mutate("aet02_1"))()


standard filter and mutation

waddella · 2021-11-18T11:40:49Z

R/aet02.R

+#'
+#' aet02_1(db_m) %>% head()
+#'
+aet02_1 <- function(adam_db,


now functions require an adam_db argument which is either a named list or a dm object

waddella · 2021-11-18T11:41:07Z

R/aet02.R

@@ -41,28 +58,20 @@ aet02_1 <- function(adsl, adae,
                      lbl_overall = ""


I still think this should be NA or NULL

NULL seems to be good

waddella · 2021-11-18T11:42:18Z

R/aet02.R

-                        lbl_AEBODSYS = "AEBODSYS",
-                        lbl_AEDECOD = "AEDECOD",
+                        lbl_AEBODSYS = "Body System or Organ Class",
+                        lbl_AEDECOD = "Dictionary-Derived Term",


this can be taken from the standards, i.e. check on bee with

library(rgdsr) rgdsr::adamv_var_info("AEBODSYS")

waddella · 2021-11-18T11:45:00Z

R/aet02.R

-    lbl_AEDECOD = lbl_AEDECOD,
+    lbl_AEBODSYS = var_labels_for(adae, "AEBODSYS"),
+    lbl_AEHLT = var_labels_for(adae, "AEHLT"),
+    lbl_AEDECOD = var_labels_for(adae, "AEDECOD"),


as these labels are nowhere else used

why do we keep these parameters in the layout function? Is it because we expect users can use the layout function if the main template function doesn't meet the need?

waddella · 2021-11-18T11:45:48Z

R/assertthat.R

@@ -0,0 +1,11 @@
+


please have a look at this code

waddella · 2021-11-18T11:46:05Z

R/dmt01.R

@@ -38,7 +43,7 @@
 #'         summaryvars = c("AGE", "RACE", "SEX"),
 #'         summaryvars_lbls = c("Age (yr)", "Race", "Sex"))
 #'
-dmt01_1 <- function(ad_bl,
+dmt01_1 <- function(data, dataname = "adsl",


to be discussed this afternoon

Suggested change

dmt01_1 <- function(data, dataname = "adsl",

dmt01_1 <- function(adam_db, dataname = "adsl",

how baseline information from dataset adsub is addressed in adam_db? In study level preprocessing, or in chevron?

waddella · 2021-11-18T11:47:19Z

R/standard_data_manipulation.R

+  "aet02_1",     "filter_adae_anl01fl",             NA,                    c("adsl", "adae"),
+  "aet02_2",     "filter_adae_anl01fl",             NA,                    c("adsl", "adae"),
+  "aet02_3",     "filter_adae_anl01fl",             NA,                    c("adsl", "adae")
+)


every new chevron function needs an entry here

waddella · 2021-11-18T11:47:57Z

R/utils.R

+ifelse_layout <- function(lyt, test, fun_lyt_yes  = identity, fun_lyt_no = identity) {
+
+  assert_that(length(test) == 1, is.logical(test))
+
+  if (test)
+    fun_lyt_yes(lyt)
+  else
+    fun_lyt_no(lyt)
+}
+
+lyt_fun <- function(fun, ...) {
+  function(lyt) {
+    fun(lyt, ...)
+  }
+}


Suggested change

ifelse_layout <- function(lyt, test, fun_lyt_yes = identity, fun_lyt_no = identity) {

assert_that(length(test) == 1, is.logical(test))

if (test)

fun_lyt_yes(lyt)

else

fun_lyt_no(lyt)

}

lyt_fun <- function(fun, ...) {

function(lyt) {

fun(lyt, ...)

}

}

it's not needed anymore

wangh107 · 2021-11-18T21:46:46Z

R/utils.R

+  db_m <- db %>%
+    dm_zoom_to(adae) %>%
+    mutate(ANL01FL = 'Y') %>%


why do we need this code in syn_test_data?

wangh107 · 2021-11-18T21:51:46Z

R/utils.R

+  db <- new_dm(sd) %>%
+     dm_add_pk(adsl, c("USUBJID", "STUDYID")) %>%
+     dm_add_fk(adae, c("USUBJID", "STUDYID"), ref_table = "adsl") %>%
+     dm_add_pk(adae, "AESEQ")


should it be dm_add_pk(adae, c("STUDYID", "USUBJID", "AESEQ")?

…refactor

adapt aet03 adapt aet04 adapt dmt01_1 adapt dst01 adapt ext01

eliminate intermediate variables rename arg

benoit and others added 11 commits November 2, 2021 15:54

refactor arguments organisation

c66b671

linting

linter correction

7c28f55

Document

fc1aa00

add_existing_table

32f35dc

Merge branch '79_split-gen_args_spec_study@main' into 84_maintain-tit…

a362a50

…le-subtitle-and-footnotes@main

linting

7e49f6c

add a setfooter function as a temporary fix

5dc770f

linting

dfb217e

add return description

e65d5d2

work in progress

92c9087

work on aet02.R

3fd0e7b

waddella requested review from wangh107 and BFalquet November 18, 2021 11:34

waddella commented Nov 18, 2021

View reviewed changes

wangh107 reviewed Nov 18, 2021

View reviewed changes

benoit and others added 11 commits November 19, 2021 17:34

Merge branch '84_maintain-title-subtitle-and-footnotes@main' into 94_…

77e459e

…refactor

bugfix

63241fe

adapt aet03 adapt aet04 adapt dmt01_1 adapt dst01 adapt ext01

discussion with Adrian

77256aa

add filter for adex

fc88bcb

changes as discussed in the meeting

265d94a

simplify layout function

0ea94ef

eliminate intermediate variables rename arg

as per discussion

12f33ba

rename args

4381b71

remove aet01_1

a8fac2b

fix documentation problem

db366a0

fix linter

0976b56

BFalquet approved these changes Nov 26, 2021

View reviewed changes

BFalquet marked this pull request as ready for review November 29, 2021 10:23

benoit added 3 commits November 29, 2021 11:41

fix typo

d440091

fix name error

f576cdf

restore url

ca282f1

BFalquet approved these changes Nov 29, 2021

View reviewed changes

include DiagrammeRsvg and DiagrammeR

d2f647e

BFalquet merged commit b8a0c97 into main Nov 29, 2021

BFalquet deleted the 94_refactor branch November 29, 2021 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

94 refactor #95

94 refactor #95

waddella commented Nov 18, 2021

github-actions bot commented Nov 18, 2021 •

edited

Loading

waddella left a comment

waddella Nov 18, 2021

waddella Nov 18, 2021

waddella Nov 18, 2021

waddella Nov 18, 2021

wangh107 Nov 18, 2021

waddella Nov 18, 2021

waddella Nov 18, 2021

wangh107 Nov 18, 2021

waddella Nov 18, 2021

waddella Nov 18, 2021

wangh107 Nov 18, 2021 •

edited

Loading

waddella Nov 18, 2021

waddella Nov 18, 2021

waddella Nov 18, 2021

wangh107 Nov 18, 2021

wangh107 Nov 18, 2021 •

edited

Loading

		@@ -41,28 +58,20 @@ aet02_1 <- function(adsl, adae,
		lbl_overall = ""

	dmt01_1 <- function(data, dataname = "adsl",
	dmt01_1 <- function(adam_db, dataname = "adsl",

94 refactor #95

94 refactor #95

Conversation

waddella commented Nov 18, 2021

github-actions bot commented Nov 18, 2021 • edited Loading

waddella left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangh107 Nov 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangh107 Nov 18, 2021 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Nov 18, 2021 •

edited

Loading

wangh107 Nov 18, 2021 •

edited

Loading

wangh107 Nov 18, 2021 •

edited

Loading