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

arbitrary horizontal lines to tm_g_gh_spaghettiplot #41

Merged
merged 28 commits into from
Sep 16, 2021

Conversation

denisovan31415
Copy link
Contributor

@denisovan31415 denisovan31415 commented Sep 9, 2021

closes #39

to test:

library(dplyr)
library(scda)
devtools::load_all()

# original ARM value = dose value
arm_mapping <- list("A: Drug X" = "150mg QD",
                    "B: Placebo" = "Placebo",
                    "C: Combination" = "Combination")

ADSL <- synthetic_cdisc_data("latest")$adsl
ADLB <- synthetic_cdisc_data("latest")$adlb
var_labels <- lapply(ADLB, function(x) attributes(x)$label)
ADLB <- ADLB %>%
  mutate(AVISITCD = case_when(
    AVISIT == "SCREENING" ~ "SCR",
    AVISIT == "BASELINE" ~ "BL",
    grepl("WEEK", AVISIT) ~ paste("W", stringr::str_extract(AVISIT, "(?<=(WEEK ))[0-9]+")),
    TRUE ~ as.character(NA)),
    AVISITCDN = case_when(
      AVISITCD == "SCR" ~ -2,
      AVISITCD == "BL" ~ 0,
      grepl("W", AVISITCD) ~ as.numeric(gsub("[^0-9]*", "", AVISITCD)),
      TRUE ~ as.numeric(NA)),
    AVISITCD = factor(AVISITCD) %>% reorder(AVISITCDN),
    TRTORD = case_when(
      ARMCD == "ARM C" ~ 1,
      ARMCD == "ARM B" ~ 2,
      ARMCD == "ARM A" ~ 3),
    ARM = as.character(arm_mapping[match(ARM, names(arm_mapping))]),
    ARM = factor(ARM) %>% reorder(TRTORD),
    ACTARM = as.character(arm_mapping[match(ACTARM, names(arm_mapping))]),
    ACTARM = factor(ACTARM) %>% reorder(TRTORD),
    hline_1 = 78,
    hline_2 = 50)
attr(ADLB[["ARM"]], "label") <- var_labels[["ARM"]]
attr(ADLB[["ACTARM"]], 'label') <- var_labels[["ACTARM"]]

app <- teal::init(
  data = cdisc_data(
    cdisc_dataset("ADSL", ADSL, code = "ADSL <- synthetic_cdisc_data(\"latest\")$adsl"),
    cdisc_dataset(
      "ADLB",
      ADLB,
      code = "ADLB <- synthetic_cdisc_data(\"latest\")$adlb
              var_labels <- lapply(ADLB, function(x) attributes(x)$label)
              ADLB <- ADLB %>%
                mutate(AVISITCD = case_when(
                    AVISIT == 'SCREENING' ~ 'SCR',
                    AVISIT == 'BASELINE' ~ 'BL',
                    grepl('WEEK', AVISIT) ~
                      paste('W', stringr::str_extract(AVISIT, '(?<=(WEEK ))[0-9]+')),
                    TRUE ~ as.character(NA)),
                  AVISITCDN = case_when(
                    AVISITCD == 'SCR' ~ -2,
                    AVISITCD == 'BL' ~ 0,
                    grepl('W', AVISITCD) ~ as.numeric(gsub('[^0-9]*', '', AVISITCD)),
                    TRUE ~ as.numeric(NA)),
                  AVISITCD = factor(AVISITCD) %>% reorder(AVISITCDN),
                  TRTORD = case_when(
                    ARMCD == 'ARM C' ~ 1,
                    ARMCD == 'ARM B' ~ 2,
                    ARMCD == 'ARM A' ~ 3),
                  ARM = as.character(arm_mapping[match(ARM, names(arm_mapping))]),
                  ARM = factor(ARM) %>% reorder(TRTORD),
                  ACTARM = as.character(arm_mapping[match(ACTARM, names(arm_mapping))]),
                  ACTARM = factor(ACTARM) %>% reorder(TRTORD),
                  hline_1 = 78,
                  hline_2 = 50)
               attr(ADLB[['ARM']], 'label') <- var_labels[['ARM']]
               attr(ADLB[['ACTARM']], 'label') <- var_labels[['ACTARM']]",
      vars = list(arm_mapping = arm_mapping)),
    check = TRUE
    ),
  modules = root_modules(
    tm_g_gh_spaghettiplot(
      label = "Spaghetti Plot",
      dataname = "ADLB",
      param_var = "PARAMCD",
      param = choices_selected(c("ALT", "CRP", "IGA"), "ALT"),
      idvar = "USUBJID",
      xaxis_var = choices_selected(c("Analysis Visit Code" = "AVISITCD"), "AVISITCD"),
      yaxis_var = choices_selected(c("AVAL","CHG", "PCHG"), "AVAL"),
      filter_var = choices_selected(c("None" = "NONE", "Screening" = "BASE2", "Baseline" = "BASE"),
       "NONE"),
      trt_group = choices_selected(c("ARM", "ACTARM"), "ARM"),
      color_comb = "#39ff14",
      man_color = c('Combination' = "#000000",
                   'Placebo' = "#fce300",
                   '150mg QD' = "#5a2f5f"),
      hline_vars = c("hline_1", "hline_2"),
      hline_vars_colors = c("pink", "yellow"),
      hline_vars_labels = c("Top", "Bottom")
    )
  )
)

shinyApp(app$ui, app$server)

Screen Shot 2021-09-09 at 6 22 42 AM

@denisovan31415 denisovan31415 added the enhancement New feature or request label Sep 9, 2021
@npaszty
Copy link
Contributor

npaszty commented Sep 9, 2021

@junlueZH
this looks great!

The pull down for the horizontal lines is going to be populated by the ANRLO, ANRHI, LLOQN and ULOQN variables. These variables all have labels and those labels would be used for the legend labels.
In the example you are passing labels via the hline_var_labels argument which wouldn't be needed if this functionality was purely driven by the variable labels. I tested this by adding labels to the two variables and commenting out the label argument and that worked too. 👍

So the only remaining item is to bring the arbitrary hline out of the plot aesthetic section and label these updated UIs per the mock I provided in issue #2

good progress, I think we're almost there.

@denisovan31415
Copy link
Contributor Author

@npaszty

implemented:

Screen Shot 2021-09-09 at 10 17 39 AM

@npaszty
Copy link
Contributor

npaszty commented Sep 9, 2021

@junlueZH
need a free text field that would be the label for the arbitrary line. the three goshawk arguments for the arbitrary horizontal line are the following so the label has to come from the UI.
hline_arb = NULL,
hline_arb_color = "red",
hline_arb_label = NULL,

please refer to the mockup in issue #2

@denisovan31415
Copy link
Contributor Author

@npaszty

Screen Shot 2021-09-09 at 3 27 53 PM

@kpagacz
Copy link
Contributor

kpagacz commented Sep 10, 2021

What is "spaigeitti"?

@denisovan31415 denisovan31415 changed the title 39 arb hori lines spaigeitti@main arbitrary horizontal lines to tm_g_gh_spaghettiplot Sep 10, 2021
@gogonzo gogonzo self-assigned this Sep 10, 2021
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

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

LGTM - I leave this PR to @npaszty supervision

@npaszty
Copy link
Contributor

npaszty commented Sep 10, 2021

@junlueZH
not I see the arbitrary line label UI but when I add text it does not update the legend.

I don't see the update to the teal module function arguments that include the arguments associated with the arbitrary line.
At this point it would be much easier for testing if the roxygen example was updated so that when I pull and build we're working off data and examples and functions within the module code itself.

@denisovan31415
Copy link
Contributor Author

denisovan31415 commented Sep 10, 2021

@npaszty

Screen Shot 2021-09-10 at 11 28 31 AM

Is this what you are seeing if you run this app?

library(dplyr)
library(scda)
devtools::load_all("~/nest_projects/goshawk/")
devtools::load_all()

# original ARM value = dose value
arm_mapping <- list("A: Drug X" = "150mg QD",
                    "B: Placebo" = "Placebo",
                    "C: Combination" = "Combination")

ADSL <- synthetic_cdisc_data("latest")$adsl
ADLB <- synthetic_cdisc_data("latest")$adlb
var_labels <- lapply(ADLB, function(x) attributes(x)$label)
ADLB <- ADLB %>%
  mutate(AVISITCD = case_when(
    AVISIT == "SCREENING" ~ "SCR",
    AVISIT == "BASELINE" ~ "BL",
    grepl("WEEK", AVISIT) ~ paste("W", stringr::str_extract(AVISIT, "(?<=(WEEK ))[0-9]+")),
    TRUE ~ as.character(NA)),
    AVISITCDN = case_when(
      AVISITCD == "SCR" ~ -2,
      AVISITCD == "BL" ~ 0,
      grepl("W", AVISITCD) ~ as.numeric(gsub("[^0-9]*", "", AVISITCD)),
      TRUE ~ as.numeric(NA)),
    AVISITCD = factor(AVISITCD) %>% reorder(AVISITCDN),
    TRTORD = case_when(
      ARMCD == "ARM C" ~ 1,
      ARMCD == "ARM B" ~ 2,
      ARMCD == "ARM A" ~ 3),
    ARM = as.character(arm_mapping[match(ARM, names(arm_mapping))]),
    ARM = factor(ARM) %>% reorder(TRTORD),
    ACTARM = as.character(arm_mapping[match(ACTARM, names(arm_mapping))]),
    ACTARM = factor(ACTARM) %>% reorder(TRTORD),
    hline_1 = 78,
    hline_2 = 50)
attr(ADLB[["ARM"]], "label") <- var_labels[["ARM"]]
attr(ADLB[["ACTARM"]], 'label') <- var_labels[["ACTARM"]]

app <- teal::init(
  data = cdisc_data(
    cdisc_dataset("ADSL", ADSL, code = "ADSL <- synthetic_cdisc_data(\"latest\")$adsl"),
    cdisc_dataset(
      "ADLB",
      ADLB,
      code = "ADLB <- synthetic_cdisc_data(\"latest\")$adlb
              var_labels <- lapply(ADLB, function(x) attributes(x)$label)
              ADLB <- ADLB %>%
                mutate(AVISITCD = case_when(
                    AVISIT == 'SCREENING' ~ 'SCR',
                    AVISIT == 'BASELINE' ~ 'BL',
                    grepl('WEEK', AVISIT) ~
                      paste('W', stringr::str_extract(AVISIT, '(?<=(WEEK ))[0-9]+')),
                    TRUE ~ as.character(NA)),
                  AVISITCDN = case_when(
                    AVISITCD == 'SCR' ~ -2,
                    AVISITCD == 'BL' ~ 0,
                    grepl('W', AVISITCD) ~ as.numeric(gsub('[^0-9]*', '', AVISITCD)),
                    TRUE ~ as.numeric(NA)),
                  AVISITCD = factor(AVISITCD) %>% reorder(AVISITCDN),
                  TRTORD = case_when(
                    ARMCD == 'ARM C' ~ 1,
                    ARMCD == 'ARM B' ~ 2,
                    ARMCD == 'ARM A' ~ 3),
                  ARM = as.character(arm_mapping[match(ARM, names(arm_mapping))]),
                  ARM = factor(ARM) %>% reorder(TRTORD),
                  ACTARM = as.character(arm_mapping[match(ACTARM, names(arm_mapping))]),
                  ACTARM = factor(ACTARM) %>% reorder(TRTORD),
                  hline_1 = 78,
                  hline_2 = 50)
               attr(ADLB[['ARM']], 'label') <- var_labels[['ARM']]
               attr(ADLB[['ACTARM']], 'label') <- var_labels[['ACTARM']]",
      vars = list(arm_mapping = arm_mapping)),
    check = TRUE
    ),
  modules = root_modules(
    tm_g_gh_spaghettiplot(
      label = "Spaghetti Plot",
      dataname = "ADLB",
      param_var = "PARAMCD",
      param = choices_selected(c("ALT", "CRP", "IGA"), "ALT"),
      idvar = "USUBJID",
      xaxis_var = choices_selected(c("Analysis Visit Code" = "AVISITCD"), "AVISITCD"),
      yaxis_var = choices_selected(c("AVAL","CHG", "PCHG"), "AVAL"),
      filter_var = choices_selected(c("None" = "NONE", "Screening" = "BASE2", "Baseline" = "BASE"),
       "NONE"),
      trt_group = choices_selected(c("ARM", "ACTARM"), "ARM"),
      color_comb = "#39ff14",
      man_color = c('Combination' = "#000000",
                   'Placebo' = "#fce300",
                   '150mg QD' = "#5a2f5f")
      # hline_vars = c("hline1: Label for hline_1" = "hline_1", "hline2: Label for hline_2" = "hline_2"),
      # hline_vars_colors = c("pink", "yellow"),
      # hline_vars_labels = c("Top", "Bottom")
    )
  )
)

shinyApp(app$ui, app$server)

@npaszty
Copy link
Contributor

npaszty commented Sep 10, 2021

I see the UIs associated to arbitrary line but when I type the label nothing goes into the legend. so yes on the UI and no on the update to legend.
how would this work if there is no reference to the arbitrary line arguments in the function?
image

@denisovan31415
Copy link
Contributor Author

denisovan31415 commented Sep 10, 2021

@npaszty

The latest commit fixes the mean / media label not displaying

Screen Shot 2021-09-10 at 2 16 58 PM

@denisovan31415
Copy link
Contributor Author

Are you referring to this range ui? still there?

Screen Shot 2021-09-10 at 2 20 58 PM

@npaszty
Copy link
Contributor

npaszty commented Sep 10, 2021

@junlueZH

seems like there are a lot of iterations and things are breaking.

  1. now the stats line and label work
  2. still no range UI
  3. the arbitrary hline is broken now
    image

are we testing against the same code?
would be helpful if you tested all the pieces together each time so that when you push an update one item isn't fixed and others broken. happy to hop on hangout to chat further.

@denisovan31415
Copy link
Contributor Author

  1. Indeed there was a name reference bug. The latest commit fixes that:
    Screen Shot 2021-09-10 at 2 41 57 PM

@denisovan31415
Copy link
Contributor Author

2 regarding the range UI, can you first confirm that this is what you are referring to?:

image

Comment on lines +195 to +211
if (!is.null(hline_vars)) {
stopifnot(is_character_vector(hline_vars, min_length = 1))
if (!is.null(hline_vars_labels)) {
stopifnot(is_character_vector(
hline_vars_labels, min_length = length(hline_vars),
max_length = (length(hline_vars)))
)
}
if (!is.null(hline_vars_colors)) {
stopifnot(is_character_vector(
hline_vars_colors,
min_length = length(hline_vars),
max_length = (length(hline_vars)))
)
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

also, it makes sense to validate arguments in the order as they are in function definition so those should be right after validations on plot_width

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented

denisovan31415 and others added 3 commits September 14, 2021 13:43
added ULOQ and LLOQ values to example data
added ULOQN and LLOQN variables to UI to complete the example
@npaszty
Copy link
Contributor

npaszty commented Sep 14, 2021

@junlueZH

made some mods and pushed. things look good now for app but one issue remains with the check = TRUE value and introduction of the LLOQ and ULOQ variables in the example data.

I don't know why check = TRUE fails so changed to check = FALSE. Can you take a look for me. code looks the same between the example data and code in the app.

@denisovan31415
Copy link
Contributor Author

@npaszty

It was failing the reproducibility check because the code was generating random numbers without setting a seed.

working now:

Screen Shot 2021-09-15 at 5 58 22 AM

@kpagacz
Copy link
Contributor

kpagacz commented Sep 15, 2021

Can we avoid generating random numbers in tests? If we pass a seed anyway, it doesn't really matter, we might as well use a constant.

@denisovan31415
Copy link
Contributor Author

Can we avoid generating random numbers in tests? If we pass a seed anyway, it doesn't really matter, we might as well use a constant.

implemented

@npaszty

If you wish to use the random numbers, then the last commit can be reverted.

@npaszty
Copy link
Contributor

npaszty commented Sep 15, 2021

@junlueZH @kpagacz
the sampling was to sample records randomly within groups of PARAMCD.
what the code looks like now is that it just picks a couple of records?

the objective is to have a few records within PARAMCD that contain a "<" or ">" so that we can test and provide example to users that illustrates the LOQs. however you want to accomplish that is fine by me.

@denisovan31415
Copy link
Contributor Author

@npaszty

The final decision is up to you. Do you want to revert the last commit and use the random numbers? That could easily be done. Please let me know.

@npaszty
Copy link
Contributor

npaszty commented Sep 15, 2021

@junlueZH @kpagacz

Something looks odd after my review.
When running the following it looks like the join is only joining the first record
ADLB %>% filter(grepl(">", LBSTRESC)) %>% with(table(PARAMCD, LBSTRESC))
LBSTRESC
PARAMCD > 73
ALT 1
CRP 0
IGA 0

The LLUQC/N and ULOQC/N values should be the same for all records of the assay. so the count should be 2800 not 1 for ALT

The example works fine and we can keep that in place though it is now different than goshawk.
is there a join that would join the one record in the ALB_LOQS to all the ALT records in the ADLB? thought left_join would do that and thought I verified in goshawk. not a show stopper but that's the intension.

@denisovan31415
Copy link
Contributor Author

@npaszty

I have reverted the last commit to use the random numbers.

This is the output of running the line of code in your comment:

> ADLB %>% filter(grepl(">", LBSTRESC)) %>% with(table(PARAMCD, LBSTRESC))
       LBSTRESC
PARAMCD > 70 > 71 > 73
    ALT    0    0    7
    CRP    7    0    0
    IGA    0    7    0

@npaszty
Copy link
Contributor

npaszty commented Sep 16, 2021

@junlueZH
okay then we'll go with this now as the prototype to be applied to boxplot and correlation plot.
need to update the goshawk functions to call the renamed helper functions (h_*) then the modules can be updated and I can review all this stuff.

thanks for your help.

@denisovan31415
Copy link
Contributor Author

@junlueZH
okay then we'll go with this now as the prototype to be applied to boxplot and correlation plot.
need to update the goshawk functions to call the renamed helper functions (h_*) then the modules can be updated and I can review all this stuff.

thanks for your help.

All of the goshawk functions have already been updated to call the (h_*) function. It was failing automation without the update. So I already did that before I merge your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

arbitrary horizontal lines (their label and color) to tm_g_gh_spaghettiplot
5 participants