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

cbind_rtables displays column names in wrong order in some cases when hierarchy structure differs #111

Closed
danielinteractive opened this issue Oct 21, 2020 · 3 comments
Assignees
Labels
Projects

Comments

@danielinteractive
Copy link
Contributor

It seems like cbind_rtables can only reliably work when used in specific order.

Example:

library(rtables)

tab1 <- rtable(
  header = "a",
  rrow("one", 1)
)
tab2 <- rtable(
  header = "b",
  rrow("one", 2)
)
tab3 <- rtable(
  header = "c",
  rrow("one", 3)
)

first <- cbind_rtables(tab1, tab2)
wrong <- cbind_rtables(first, tab3)
ok <- cbind_rtables(tab3, first)

Here wrong has the wrong order of column names (but cell content is correct). ok is ok.

Session info:

R version 3.6.3 (2020-02-29)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Debian GNU/Linux 9 (stretch)

Matrix products: default
BLAS/LAPACK: /usr/lib/libopenblasp-r0.2.19.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8       
 [4] LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8    LC_MESSAGES=C             
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                  LC_ADDRESS=C              
[10] LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] rtables_0.3.2.17.9029 magrittr_1.5         

loaded via a namespace (and not attached):
[1] compiler_3.6.3  htmltools_0.4.0 tools_3.6.3     Rcpp_1.0.4.6    digest_0.6.25   packrat_0.5.0  
[7] rlang_0.4.5     purrr_0.3.4    
@danielinteractive danielinteractive changed the title cbind_rtables gives wrong order when applied twice cbind_rtables gives wrong column name order when applied twice Oct 21, 2020
@danielinteractive danielinteractive added this to ToDo in TableTrees via automation Oct 21, 2020
@danielinteractive
Copy link
Contributor Author

Thanks @gmbecker for the workaround and generalization of cbind_rtables(), that works nicely!
I guess this bug would still be good to get fixed though, since it will be very confusing for users.

@gmbecker gmbecker changed the title cbind_rtables gives wrong column name order when applied twice cbind_rtables displays column names in wrong order in some cases when hierarchy structure differs Oct 22, 2020
@anajens
Copy link
Contributor

anajens commented Oct 27, 2020

Some more examples of this bug:

library(rtables)

df1 <- data.frame(
  arm = c("ARM A", "ARM B"),
  stat2 = c(2, 4),
  stat3 = c(3, 5)
) 

df2 <- data.frame(
  arm = c("OVERALL", "OVERALL"),
  stat3 = 6,
  stat4 = 7,
  stat1 = 1
) 

t1 <- basic_table() %>%
  split_cols_by("arm") %>%
  split_cols_by_multivar(vars = c("stat2", "stat3")) %>%
  analyze_colvars(afun = mean) %>%
  build_table(df1)

t2 <- basic_table() %>%
  split_cols_by("arm") %>%
  split_cols_by_multivar(vars = c("stat3", "stat4", "stat1")) %>%
  analyze_colvars(afun = mean) %>%
  build_table(df2)

# Works:
cbind_rtables(t1, t2)

# Here however, there is a warning and header is incorrect:
cbind_rtables(t2[, 3], t1, t2[, 1:2])

# Also with cbind in 2 steps gives yet another result for the header:
cbind_rtables(
  cbind_rtables(t2[, 3], t1),
  t2[, 1:2]
)

Session info:

R version 3.6.3 (2020-02-29)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Debian GNU/Linux 9 (stretch)

Matrix products: default
BLAS/LAPACK: /usr/lib/libopenblasp-r0.2.19.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8    LC_MESSAGES=C             
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] rtables_0.3.2.17.9035 magrittr_1.5         

loaded via a namespace (and not attached):
[1] compiler_3.6.3  htmltools_0.4.0 tools_3.6.3     Rcpp_1.0.4.6    digest_0.6.25   packrat_0.5.0   rlang_0.4.5     purrr_0.3.4

@gmbecker
Copy link
Collaborator

Fixed the underlying bug so that the initial recursive cbinding displays correctly.

TableTrees automation moved this from ToDo to Done Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
TableTrees
  
Done
Development

No branches or pull requests

3 participants