Initial kmg01_1 #484

Teninq · 2023-04-13T07:29:10Z

close insightsengineering/chevron-tasks#66

github-actions · 2023-04-13T07:38:59Z

Unit Tests Summary

    1 files   33 suites 1m 38s ⏱️
178 tests 128 ✔️   50 💤 0 ❌
348 runs 234 ✔️ 114 💤 0 ❌

Results for commit d85e3de.

♻️ This comment has been updated with latest results.

github-actions · 2023-04-13T14:39:20Z

🧪 $Test coverage: 97.13%$

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  --------------------------------------------------------------
R/ael01_nollt.R                 26       2  92.31%   82-91
R/aet01_aesi.R                 185       7  96.22%   40, 42-46, 278
R/aet01.R                      342       5  98.54%   41, 253, 328, 334, 562
R/aet02.R                      235       1  99.57%   494
R/aet03.R                       83       0  100.00%
R/aet04.R                      106       2  98.11%   169, 186
R/aet10.R                       50       1  98.00%   102
R/assertions.R                  71       5  92.96%   178-182
R/checks.R                      20       0  100.00%
R/chevron_tlg-S4class.R         21       0  100.00%
R/chevron_tlg-S4methods.R      136      12  91.18%   410-482
R/cmt01a.R                     186       0  100.00%
R/cmt02_pt.R                    49       0  100.00%
R/dmt01.R                       30       0  100.00%
R/dst01.R                      290       0  100.00%
R/dtht01.R                      98       0  100.00%
R/egt01.R                       45       0  100.00%
R/egt02.R                       56       0  100.00%
R/egt03.R                      132       3  97.73%   118, 164, 322
R/egt05_qtcat.R                 57       0  100.00%
R/ext01.R                       71       4  94.37%   230-231, 235-236
R/kmg01.R                       37       7  81.08%   58-66, 106
R/lbt01.R                       94       0  100.00%
R/lbt04.R                       49       0  100.00%
R/lbt05.R                       68       5  92.65%   148-153
R/lbt07.R                       90       1  98.89%   171
R/lbt14.R                      188      24  87.23%   54, 56, 108-109, 111-117, 144, 257, 259, 311-312, 314-320, 347
R/mht01.R                       69       2  97.10%   33-34
R/mng01.R                       95      12  87.37%   114, 118-121, 130-138, 180
R/pdt01.R                       57       0  100.00%
R/pdt02.R                       65       0  100.00%
R/utils.R                       73       0  100.00%
R/vst01.R                       47       0  100.00%
R/vst02.R                       92       2  97.83%   109, 240
TOTAL                         3313      95  97.13%

Diff against main

Filename          Stmts    Miss  Cover
--------------  -------  ------  -------
R/assertions.R       +7      +5  -7.04%
R/kmg01.R           +37      +7  +81.08%
TOTAL               +44     +12  -0.33%

Results for commit: fb170f91f624b1490f18f691ebdea3a16fa47c87

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Teninq · 2023-04-14T07:24:53Z

Create template from issue: 373

clarkliming · 2023-04-14T08:51:39Z

R/kmg01.R

+                         ...) {
+  anl <- adam_db[[dataset]]
+  checkmate::assert_true(length(unique(anl$PARAMCD)) == 1)
+  checkmate::assert_character(x_name)


use assert_string

clarkliming · 2023-04-14T08:53:59Z

R/kmg01.R

+  }
+
+  gkm_plot <- if (!show_statis) {
+    g_km(


these two parts can be put together

clarkliming · 2023-04-14T08:54:16Z

R/kmg01.R

+  checkmate::assert_class(adam_db, "dm")
+  assert_colnames(adam_db[[dataset]], c("PARAMCD", "CNSR"))
+
+  adam_db <- adam_db %>%


use list of data frames

R/kmg01.R

clarkliming · 2023-04-20T03:28:36Z

R/kmg01.R

+#' @examples
+#' library(dplyr)
+#'
+#' col <- c(


to make it consistent with the argument passing from citril, we have to make it either a named list or unnamed character

Changed with list()

clarkliming · 2023-04-20T04:32:25Z

R/kmg01.R

+    col <- line_col
+  }
+
+  gkm_plot <- g_km(


no need to assign it to gkm_plot as it is returned directly

…eering/chevron into 373_kmg01_template

clarkliming · 2023-04-21T03:30:02Z

R/utils.R

@@ -266,3 +266,16 @@ gg_list <- function(...) {
    class = c("gg_list", "list")
  )
 }
+
+#' Check to have only one PARAMCD in the analysis dataset


this assertion check put in assertions.R

clarkliming · 2023-04-21T03:30:32Z

R/utils.R

@@ -266,3 +266,16 @@ gg_list <- function(...) {
    class = c("gg_list", "list")
  )
 }
+
+#' Check to have only one PARAMCD in the analysis dataset
+#' @param param_val unique value of PARAMCD


add type hint of the param_val (character)

clarkliming · 2023-04-21T03:31:38Z

R/utils.R

+  if (length(param_val) > 1) {
+    stop(paste0(
+      "More than one parameters:",
+      paste(param_val, collapse = ", "),


use toString(param_val)

clarkliming · 2023-04-21T03:32:00Z

R/utils.R

+#' @param param_val unique value of PARAMCD
+#' @export
+assert_only_one_paramcd <- function(param_val) {
+  if (length(param_val) > 1) {


check for input values

clarkliming · 2023-04-21T03:32:20Z

R/utils.R

+    stop(paste0(
+      "More than one parameters:",
+      paste(param_val, collapse = ", "),
+      ", Only one suppose to have."


after "," should not use up case

R/kmg01.R

clarkliming · 2023-04-21T03:35:33Z

R/kmg01.R

+  checkmate::assert_character(line_col, null.ok = TRUE)
+
+  assert_colnames(adam_db[[dataset]], "AVAL")
+  variables <- list(tte = "AVAL", is_event = "is_event", arm = arm_var)


this is not suitable to have "AVAL" and "is_event" fixed here. I would rather expose these two arguments. And, should not we use CNSR here (0 for event and 1 for censor)

Keep it for now

but we should check if is_event exists in data

and also arm_var

clarkliming · 2023-04-21T07:40:28Z

R/assertions.R

+
+
+#' Check to have only one PARAMCD in the analysis dataset
+#' @param param_val (`character`) value of PARAMCD


since we don't require it to be a character (as factor also works), remove the type hint or at least check if it is character or factor, and correct the type hint

clarkliming

looks good, thank you

Initial kmg01_1

138bbcd

Chenkai Lv and others added 5 commits April 13, 2023 08:17

solve checks

eaea7cd

Update wordlist

d146c6e

Update pkgdown

beccf85

Add arguments

00d8804

Merge branch 'main' into 373_kmg01_template

4f1faa7

Chenkai Lv added 3 commits April 13, 2023 15:03

Add new checks and update document

a554eb4

Add line breaker

68dd797

Update wordlist

288dc87

clarkliming reviewed Apr 14, 2023

View reviewed changes

R/kmg01.R Outdated

}

gkm_plot <- if (!show_statis) {

g_km(

Copy link

Contributor

clarkliming Apr 14, 2023

Choose a reason for hiding this comment

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

these two parts can be put together

clarkliming reviewed Apr 14, 2023

View reviewed changes

R/kmg01.R Show resolved Hide resolved

Teninq and others added 10 commits April 19, 2023 09:51

Merge branch 'main' into 373_kmg01_template

bfc0720

Merge branch 'main' into 373_kmg01_template

e5a4615

Update comments and documents

62c5d03

Update style

49bd8fc

Update unit test to remove legend position

dc659bb

Remove library dm

02a02bc

Merge branch 'main' into 373_kmg01_template

40ed2de

Update line widths

973cb48

add aval assertion

2584a65

Update with latest dunlin

4f5d725

clarkliming reviewed Apr 20, 2023

View reviewed changes

R/kmg01.R Show resolved Hide resolved

clarkliming reviewed Apr 20, 2023

View reviewed changes

Update testthat

a0f4129

clarkliming reviewed Apr 20, 2023

View reviewed changes

LVC11 and others added 12 commits April 20, 2023 04:44

Load dunlin

6c384bc

Update style

7265d98

update gitignore

6c9959d

Update paramcd assertion

d862ad2

Merge branch '373_kmg01_template' of https://github.com/insightsengin…

ec876c9

…eering/chevron into 373_kmg01_template

Update lint code

f0c05ba

Update style

a95703b

Add assertion to check only one paramcd exists

273e7ec

Add missing topic

1ec7a57

Add documentation to argument 'param_val'

3d5cdf1

Update style

547ce12

Update style and document

a6111e6

clarkliming reviewed Apr 21, 2023

View reviewed changes

R/kmg01.R Show resolved Hide resolved

clarkliming reviewed Apr 21, 2023

View reviewed changes

LVC11 added 3 commits April 21, 2023 07:13

Update comments

b9a2e5d

Update style

3883ebd

Update style

efded8a

clarkliming reviewed Apr 21, 2023

View reviewed changes

LVC11 added 2 commits April 24, 2023 02:34

Add check var_arm and is_event

756a8fe

Remove type hint

d85e3de

clarkliming approved these changes Apr 24, 2023

View reviewed changes

Teninq merged commit e7780b7 into main Apr 24, 2023

Teninq deleted the 373_kmg01_template branch April 24, 2023 05:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial kmg01_1 #484

Initial kmg01_1 #484

Teninq commented Apr 13, 2023 •

edited by clarkliming

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

Teninq commented Apr 14, 2023

clarkliming Apr 14, 2023

clarkliming Apr 14, 2023

clarkliming Apr 14, 2023

clarkliming Apr 20, 2023

Teninq Apr 21, 2023

clarkliming Apr 20, 2023

Teninq Apr 21, 2023

clarkliming Apr 21, 2023

Teninq Apr 21, 2023

clarkliming Apr 21, 2023

Teninq Apr 21, 2023

clarkliming Apr 21, 2023 •

edited

Loading

Teninq Apr 21, 2023

clarkliming Apr 21, 2023

clarkliming Apr 21, 2023

Teninq Apr 21, 2023

clarkliming Apr 21, 2023

Teninq Apr 21, 2023

clarkliming Apr 21, 2023

clarkliming Apr 21, 2023

clarkliming Apr 21, 2023

clarkliming left a comment



		#' Check to have only one PARAMCD in the analysis dataset
		#' @param param_val (`character`) value of PARAMCD

Initial kmg01_1 #484

Initial kmg01_1 #484

Conversation

Teninq commented Apr 13, 2023 • edited by clarkliming Loading

github-actions bot commented Apr 13, 2023 • edited Loading

Unit Tests Summary

github-actions bot commented Apr 13, 2023 • edited Loading

Code Coverage Summary

Diff against main

Teninq commented Apr 14, 2023

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

clarkliming Apr 21, 2023 • 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

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

clarkliming left a comment

Choose a reason for hiding this comment

Teninq commented Apr 13, 2023 •

edited by clarkliming

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Apr 13, 2023 •

edited

Loading

clarkliming Apr 21, 2023 •

edited

Loading