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

new argument to replace axes break points #116

Closed
wants to merge 2 commits into from

Conversation

denisovan31415
Copy link
Contributor

closes #115

test with roxygen apps

@denisovan31415 denisovan31415 added the enhancement New feature or request label Mar 9, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2022

Code Coverage Summary

Filename                           Stmts    Miss  Cover    Missing
-------------------------------  -------  ------  -------  ---------
R/g_boxplot.R                        109     109  0.00%    154-310
R/g_correlationplot.R                130     130  0.00%    260-423
R/g_density_distribution_plot.R       84      84  0.00%    129-238
R/g_lineplot.R                       246     246  0.00%    269-583
R/g_scatterplot.R                    118     118  0.00%    142-297
R/g_spaghettiplot.R                   98      98  0.00%    202-332
R/geom_axes_line.R                   190     190  0.00%    48-459
R/t_summarytable.R                    83      83  0.00%    87-205
R/utils.R                             72      72  0.00%    21-147
TOTAL                               1130    1130  0.00%

Results for commit: 81c77b6

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@npaszty
Copy link
Contributor

npaszty commented Mar 9, 2022

@junlueZH

  1. density: when replace axis is TRUE get the following warnings. not when FALSE. is this something that needs to be addressed?
    image

replace axis functionality itself looks good though

@denisovan31415
Copy link
Contributor Author

denisovan31415 commented Mar 10, 2022

@npaszty

I am not getting those warnings on my end when running density.

I am using local R Studio with the latest staged dependencies installed.

What environment are you running it on?

@denisovan31415
Copy link
Contributor Author

@npaszty

I just ran it on BEE with the latest staged dependencies:

Screen Shot 2022-03-10 at 6 05 36 AM

@kpagacz kpagacz self-assigned this Mar 11, 2022
@gogonzo gogonzo self-assigned this Mar 11, 2022
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.

line_axis_label breaks encapsulation of straight-lines and also uses %>% instead of + to add the geom. line_axis_label should be redesigned with respect to the current straight-lines implementation. We recently had an issue especially for limiting numbers of arguments and here another argument is introduced.

In my opinion teal.goshawk and end-users should use + scale_x_continuous(breaks = ...) to set breaks manually instead of growing goshawk functions. For some reason breaks are independent on the geoms in ggplot2.

@denisovan31415
Copy link
Contributor Author

denisovan31415 commented Mar 11, 2022

@gogonzo I can redesign it to use the + instead of the pipe.

However, I believe that it is unavoidable to have a new argument. Because if the end user is not expected to be able to do this:

p + coord_cartesian(...)

according to @npaszty

Then we cannot expect them to know:

+ scale_x_continuous(breaks = ...)

@gogonzo
Copy link
Contributor

gogonzo commented Mar 14, 2022

I don't know if there is much to do. Straight lines should not affect axis so we shouldn't make any hacks to display additional breaks for them. None of the geoms in ggplot (as far as I know) influence the axis. Axes are independent and they should only depend on limits. This PR was problematic and I think it should be reverted.

Also, behaviour spotted by Nick Paszty isn't something new, but in the former releases we could also see insufficient number of breaks.

2021_10_13

source("https://raw.github.roche.com/NEST/nest_on_bee/master/bee_nest_utils.R")
bee_use_nest(release = "2021_10_13")

# spaghetti ----
library(goshawk)
library(scda)
library(stringr)

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

ASL <- 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",
        trimws(
          substr(
            AVISIT,
            start = 6,
            stop = str_locate(AVISIT, "DAY") - 1
          )
        )
      ),
    TRUE ~ NA_character_
  )) %>%
  mutate(AVISITCDN = case_when(
    AVISITCD == "SCR" ~ -2,
    AVISITCD == "BL" ~ 0,
    grepl("W", AVISITCD) ~ as.numeric(gsub("\\D+", "", AVISITCD)),
    TRUE ~ NA_real_
  )) %>%
  # use ARMCD values to order treatment in visualization legend
  mutate(TRTORD = ifelse(grepl("C", ARMCD), 1,
                         ifelse(grepl("B", ARMCD), 2,
                                ifelse(grepl("A", ARMCD), 3, NA)
                         )
  )) %>%
  mutate(ARM = as.character(arm_mapping[match(ARM, names(arm_mapping))])) %>%
  mutate(ARM = factor(ARM) %>%
           reorder(TRTORD)) %>%
  mutate(ANRLO = .30, ANRHI = .75) %>%
  rowwise() %>%
  group_by(PARAMCD) %>%
  mutate(LBSTRESC = ifelse(USUBJID %in% sample(USUBJID, 1, replace = TRUE),
                           paste("<", round(runif(1, min = 25, max = 30))), LBSTRESC
  )) %>%
  mutate(LBSTRESC = ifelse(USUBJID %in% sample(USUBJID, 1, replace = TRUE),
                           paste(">", round(runif(1, min = 70, max = 75))), LBSTRESC
  )) %>%
  ungroup()
attr(ADLB[["ARM"]], "label") <- var_labels[["ARM"]]
attr(ADLB[["ANRLO"]], "label") <- "Analysis Normal Range Lower Limit"
attr(ADLB[["ANRHI"]], "label") <- "Analysis Normal Range Upper Limit"

# add LLOQ and ULOQ variables
ADLB_LOQS <- goshawk:::h_identify_loq_values(ADLB)
ADLB <- left_join(ADLB, ADLB_LOQS, by = "PARAM")

g_spaghettiplot(
  data = ADLB,
  subj_id = "USUBJID",
  biomarker_var = "PARAMCD",
  biomarker = "CRP",
  value_var = "AVAL",
  trt_group = "ARM",
  time = "AVISITCD",
  color_manual = color_manual,
  color_comb = "#39ff14",
  alpha = .02,
  xtick = c("BL", "W 1", "W 4"),
  xlabel = c("Baseline", "Week 1", "Week 4"),
  rotate_xlab = FALSE,
  group_stats = "median",
  hline_arb = 90,
  hline_vars = c("ANRHI", "ANRLO"),
  hline_vars_colors = c("pink", "brown"),
  ylim = c(10, 90)
)

image

2022_01_28

source("https://raw.github.roche.com/NEST/nest_on_bee/master/bee_nest_utils.R")
bee_use_nest(release = "2022_01_28")

# spaghetti ----
library(goshawk)
library(scda)
library(stringr)

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

ASL <- 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",
        trimws(
          substr(
            AVISIT,
            start = 6,
            stop = str_locate(AVISIT, "DAY") - 1
          )
        )
      ),
    TRUE ~ NA_character_
  )) %>%
  mutate(AVISITCDN = case_when(
    AVISITCD == "SCR" ~ -2,
    AVISITCD == "BL" ~ 0,
    grepl("W", AVISITCD) ~ as.numeric(gsub("\\D+", "", AVISITCD)),
    TRUE ~ NA_real_
  )) %>%
  # use ARMCD values to order treatment in visualization legend
  mutate(TRTORD = ifelse(grepl("C", ARMCD), 1,
                         ifelse(grepl("B", ARMCD), 2,
                                ifelse(grepl("A", ARMCD), 3, NA)
                         )
  )) %>%
  mutate(ARM = as.character(arm_mapping[match(ARM, names(arm_mapping))])) %>%
  mutate(ARM = factor(ARM) %>%
           reorder(TRTORD)) %>%
  mutate(ANRLO = .30, ANRHI = .75) %>%
  rowwise() %>%
  group_by(PARAMCD) %>%
  mutate(LBSTRESC = ifelse(USUBJID %in% sample(USUBJID, 1, replace = TRUE),
                           paste("<", round(runif(1, min = 25, max = 30))), LBSTRESC
  )) %>%
  mutate(LBSTRESC = ifelse(USUBJID %in% sample(USUBJID, 1, replace = TRUE),
                           paste(">", round(runif(1, min = 70, max = 75))), LBSTRESC
  )) %>%
  ungroup()
attr(ADLB[["ARM"]], "label") <- var_labels[["ARM"]]
attr(ADLB[["ANRLO"]], "label") <- "Analysis Normal Range Lower Limit"
attr(ADLB[["ANRHI"]], "label") <- "Analysis Normal Range Upper Limit"

# add LLOQ and ULOQ variables
ADLB_LOQS <- goshawk:::h_identify_loq_values(ADLB)
ADLB <- left_join(ADLB, ADLB_LOQS, by = "PARAM")

g_spaghettiplot(
  data = ADLB,
  subj_id = "USUBJID",
  biomarker_var = "PARAMCD",
  biomarker = "CRP",
  value_var = "AVAL",
  trt_group = "ARM",
  time = "AVISITCD",
  color_manual = color_manual,
  color_comb = "#39ff14",
  alpha = .02,
  xtick = c("BL", "W 1", "W 4"),
  xlabel = c("Baseline", "Week 1", "Week 4"),
  rotate_xlab = FALSE,
  group_stats = "median",
  hline_vars = c("ANRHI", "ANRLO"),
  hline_vars_colors = c("pink", "brown"),
  hline_arb = 1.9
)


image

I suggest to close this PR and revert recent changes in goshawk and teal.goshawk

@denisovan31415
Copy link
Contributor Author

@npaszty closing this ticket as discussed with @gogonzo.
issue needs to be moved to backlog to be refined and discussed.

@gogonzo gogonzo deleted the 115_axes_replacement_arg@main branch April 11, 2022 04:59
@gogonzo gogonzo self-assigned this May 2, 2022
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.

argument to replace axes break points with hori / vert line break points
4 participants