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

Enforce uniqueness of sibling names in row space #864

Open
gmbecker opened this issue Apr 25, 2024 · 3 comments · May be fixed by #994
Open

Enforce uniqueness of sibling names in row space #864

gmbecker opened this issue Apr 25, 2024 · 3 comments · May be fixed by #994

Comments

@gmbecker
Copy link
Collaborator

gmbecker commented Apr 25, 2024

Reporting an Issue with rtables

Normally, all paths in row space are "addressable", ie we can do

rdf <- make_rows_df(tbl)
pieces <- lapply(rdf$path, function(pth) tt_at_path(tbl, pth))

And everythig will work (we even have a test of this form in there somewhere.

Unfortunately, this is not guaranteed/enforced for all tables, specifically if we do

adsl <- ex_adsl
adsl$flag <- sample(c("Y", "N"), nrow(adsl), replace = TRUE)

afun <- function(x, ...) rcell(label = "Flagged Pop. Count", sum(x == "Y"))

lyt <- basic_table() %>%
  analyze("flag", afun = afun) %>%
  split_rows_by("flag", split_fun = keep_split_levels("Y"), child_labels = "hidden") %>%
  split_rows_by("SEX") %>%
  analyze("BMRKR1")

tbl <- build_table(lyt, adsl)

We get a table that builds and prints fine:

> tbl
                     all obs
————————————————————————————
Flagged Pop. Count     200  
F                           
  Mean                5.56  
M                           
  Mean                5.72  
U                           
  Mean                6.82  
UNDIFFERENTIATED            
  Mean                9.70

But

> lapply(make_row_df(tbl)$path, function(pth) tt_at_path(tbl, pth))
Error in tt_at_path(tbl, pth) :
Path appears invalid for this tree at step Y

In particulate, the issue is with any path starting with of the form

badpath <- c("root", "flag", "Y")
> tt_at_path(tbl, badpath)
Error in tt_at_path(tbl, badpath) : 
  Path appears invalid for this tree at step Y

We can see the reason in the table structure:

> table_structure(tbl)
[TableTree] root
 [ElementaryTable] flag (1 x 1)
 [TableTree] flag
  [TableTree] Y
   [TableTree] SEX
    [TableTree] F
     [ElementaryTable] BMRKR1 (1 x 1)
    [TableTree] M
     [ElementaryTable] BMRKR1 (1 x 1)
    [TableTree] U
     [ElementaryTable] BMRKR1 (1 x 1)
    [TableTree] UNDIFFERENTIATED
     [ElementaryTable] BMRKR1 (1 x 1)

Any path starting with c("root", "flag") is going to go into the Elementary table named flag and so it won't find any of the children of the TableTree flag. This is documented behavior, but still troublesome, so rtables should enforce siblings have unique names (the way it already does when analyzing the same variable twice).

Should be easy to detect and enforce (possibly with a warning/message) in the TableTree constructor. Note we need to be looking at and modifying obj_name of the incoming children, not just the names in the list object that holds the children.

This analyze and then split on flag variables is a pattern we are using in practice a fair bit, so the above example is quite realistic to our needs

@Melkiades
Copy link
Contributor

I think this is a bug we need to address. I can think of many sorting cases where this may appear.

@gmbecker
Copy link
Collaborator Author

gmbecker commented Feb 4, 2025

@Melkiades have you had a chance to look at this? any thoughts?

@Melkiades Melkiades self-assigned this Feb 6, 2025
@Melkiades
Copy link
Contributor

@Melkiades have you had a chance to look at this? any thoughts?

Added a fix. We need to work on it for sorting and other cases where this can happen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants