bcjaeger / tibbleOne

convenient framework to tabulate participant characteristics.
https://bcjaeger.github.io/tibbleOne/
1 stars 2 forks source link

build_meta doesn't work well with expand_binary_catgs argument #10

Open boyiguo1 opened 4 years ago

boyiguo1 commented 4 years ago

When the meta data is passed into tibble_one as an argument, the result with expand_binary_catgs would be wrong: variable label will be also used for the level labels. See example below.

library(tidyverse)
#> Warning: package 'tidyverse' was built under R version 3.6.2
#> Warning: package 'ggplot2' was built under R version 3.6.1
#> Warning: package 'tidyr' was built under R version 3.6.2
#> Warning: package 'purrr' was built under R version 3.6.1
#> Warning: package 'forcats' was built under R version 3.6.3
library(tibbleOne)

n <- 10

df <- data.frame(
  a = rep(c("X","Y"), n / 2),
  b = rep(c("A", "B"), each = n / 2)
)
# Correct labels
tibble_one(data = df,
           formula = ~ a + b,
           expand_binary_catgs = T)
#> # A tibble: 7 x 4
#>   group variable labels              Overall
#>   <fct> <fct>    <chr>               <chr>  
#> 1 None  descr    No. of observations "10"   
#> 2 None  a        A, %                ""     
#> 3 None  a        X                   "50.0" 
#> 4 None  a        Y                   "50.0" 
#> 5 None  b        B, %                ""     
#> 6 None  b        A                   "50.0" 
#> 7 None  b        B                   "50.0"

meta <- df %>% build_meta()
# Wrong labels
tibble_one(data=df,
           formula = ~ a + b,
           meta_data  = meta,
           expand_binary_catgs = T
)
#> # A tibble: 7 x 4
#>   group variable labels              Overall
#>   <fct> <fct>    <chr>               <chr>  
#> 1 None  descr    No. of observations "10"   
#> 2 None  a        A, %                ""     
#> 3 None  a        A                   "50.0" 
#> 4 None  a        A                   "50.0" 
#> 5 None  b        B, %                ""     
#> 6 None  b        B                   "50.0" 
#> 7 None  b        B                   "50.0"

Created on 2020-03-22 by the reprex package (v0.3.0)

boyiguo1 commented 4 years ago

I think this is because when build_meta is called outside tibble_one, it defaults expand_binary_catgs=F. If not carefully read the document of build_meta, its possible that people would ignore it and report issues.

I would prefer in the document, say something like build_meta is preferred to not to use external of tibble_one if necessary. or have a revision of the tutorial in the future.

bcjaeger commented 4 years ago

Good point. Do you think we should implement a fix that throws an error if expand_binary_catg is set as TRUE in tibble_one() but a build_meta() object is supplied externally where expand_binary_catg was FALSE? We could do this if we added an attribute to objects outputted by build_meta() indicating the expand_binary_catg value. I also like the idea of adding some detail to the documentation.

boyiguo1 commented 4 years ago

I would suggest a warning instead of error. I would argue that we send out an warning that the specifications of expand_binary_catg are inconsistent. Hence, we make meta again within tibbleOne and output the correct result.

The reasoning being, having to specify expand_binary_catg twice is a design flaw IMO (not in a disdainful tone). Personally, I think sending error is like penalizing people for our inconsideration. On the contrary, sending warning is like saying, you are not quiet using the way that we wanted. we corrected for you, but just want to let you be aware.

Does this make sense?

bcjaeger commented 4 years ago

Yes, it does seem like a design flaw in retrospect. I would suggest fixing the design flaw, but I think that would break backwards compatibility on CRAN. Re-creating the meta object using the arguments that were specified in tibble_one() does make sense to me. Do you want to have a zoom call tomorrow to discuss?