openpharma/visR

Bug in `define_theme(strata=)`

Closed this issue · 3 comments

I was browsing the vignettes and noticed that when the theme is defined with multiple stratum, every level is shown in the legend---even when that stratum doesn't appear in figure.

library(visR)
packageVersion("visR")
#> [1] '0.2.0.9006'

theme <-
  visR::define_theme(
    strata = list("Sex" = list("Female" = "red",
                               "Male" = "blue"),
                  "ph.ecog" = list("0" = "cyan",
                                   "1" = "purple",
                                   "2" = "brown")))

survival::lung %>%  
  dplyr::mutate(sex = as.factor(ifelse(sex == 1, "Male", "Female")))  %>%  
  dplyr::mutate(status = status - 1) %>%
  dplyr::rename(Age = "age", Sex = "sex", Status = "status", Days = "time")  %>%
  visR::estimate_KM(strata = "Sex", CNSR = "Status", AVAL = "Days") %>%
  visR::visr() %>%
  visR::apply_theme(theme)

Created on 2022-05-27 by the reprex package (v2.0.1)

Potentially add tests to catch this. Also, snapshots to help with unit testing of graphs. Also, possibly check between survfit and estimateKM object and add test if so.

https://testthat.r-lib.org/articles/snapshotting.html

The issue is that the function is not documented well and the example does not make sense:

## define new theme
theme <-
  visR::define_theme(
    strata = list("Sex" = list("Female" = "red",
                               "Male" = "blue"),
                  "ph.ecog" = list("0" = "cyan",
                                   "1" = "purple",
                                   "2" = "brown")))
## all strata used
survobj<-
survival::lung %>%
  dplyr::mutate(sex = as.factor(ifelse(sex == 1, "Male", "Female")))  %>%
  dplyr::mutate(status = status - 1) %>%
  dplyr::rename(Age = "age", Sex = "sex", Status = "status", Days = "time")  %>%
  visR::estimate_KM(strata = c("Sex", "ph.ecog"), CNSR = "Status", AVAL = "Days")

survobj%>%
  visR::visr() 

image

gg %>%
 visR::apply_theme(theme))

Note that the defaults colours of the theme_minimal are used and since they cant be mapped it defaults to grey50. The colors should have been mapped to:

as.character(unique(gg$data[[gg$labels$group]]))
[1] "Female, 0" "Female, 1" "Female, 2" "Male, 0" "Male, 1" "Male, 2" "Male, 3"

image

In the example it is assumed that we have different strata, each with a color attached. As you see, we actually have combination of the two variables for which a color needs to be defined, not the individual variable levels.

When we use a proper example, it does work:

theme <-
  visR::define_theme(
    strata = list("Sex, ph.ecog" = list("Female, 0" = "red",
                                        "Male, 0" = "blue")))
survobj<-
survival::lung %>%
  dplyr::mutate(sex = as.factor(ifelse(sex == 1, "Male", "Female")))  %>%
  dplyr::mutate(status = status - 1) %>%
  dplyr::rename(Age = "age", Sex = "sex", Status = "status", Days = "time")  %>%
  visR::estimate_KM(strata = c("Sex", "ph.ecog"), CNSR = "Status", AVAL = "Days")

survobj%>%
  visR::visr() %>%
 apply_theme(theme)

image

We should discuss the proper requirements for this function and update accordingly:

  • what happens if some strata are not present in the data but defined in the theme. Should we at least warn the user because these fake strata end up in the legend. Should we remove those strata from legend?
  • what happens if we don't give color to every strata in the data. Should we display them still in the legend?

I will make a proposal for the function.

Closed with #411