prune_table returns NULL if all rows pruned

[clarkliming]

prune_table, according to documentation, should return a TableTree object, but it could also return NULL.

I wonder if we should return the table with no contents but the header? (is it a legal rtable object?)

adsl <- ex_adsl
levels(adsl$SEX) <- c(levels(ex_adsl$SEX), "OTHER")

tbl_to_prune <- basic_table() %>%
  split_cols_by("ARM") %>% 
  split_rows_by("SEX") %>% 
  summarize_row_groups() %>%
  split_rows_by("STRATA1") %>%
  summarize_row_groups() %>%
  analyze("AGE") %>%
  build_table(adsl[-c(1:400),])

tbl_to_prune %>% prune_table

[Melkiades]

It is possible to do that with tbl_to_prune[-c(1:nrow(tbl_to_prune)),]

[clarkliming]

It is possible to do that with tbl_to_prune[-c(1:nrow(tbl_to_prune)),]

thanks. Then returning "NULL" can be inappropriate here

[Melkiades]

So your suggestion is to have prune return an empty table instead of NULL. I think it makes sense. @gmbecker were there some specific design principles behind this NULL output?

[gmbecker]

We should discuss this more. I'm not saying I disagree but at the same time a degenerate tabletree object is really not very useful in practice.

On Mon, Mar 6, 2023, 6:54 AM Davide Garolini @.***> wrote:

So your suggestion is to have prune return an empty table instead of NULL. I think it makes sense. @gmbecker https://github.com/gmbecker were there some specific design principles behind this NULL output?

— Reply to this email directly, view it on GitHub https://github.com/insightsengineering/rtables/issues/566#issuecomment-1456286345, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG53MMTCRSUDSWVDAEH6XTW2X3DVANCNFSM6AAAAAAVRAPQZI . You are receiving this because you were mentioned.Message ID: @.***>

[gmbecker]

prune_table, according to documentation, should return a TableTree object, but it could also return NULL.

I wonder if we should return the table with no contents but the header? (is it a legal rtable object?)

... Kinda. As @Melkiades points out, it IS possible to get a TableTree object with no rows in it, and it prints ok. That said, I definitely can't promise every single function that operates on TableTree objects will work (ones that operate on a table's column information should); Anything that deals in rows I would consider to have undefined behavior currently for a degenerate table. I think most of it should work, but it's certainly not robustly tested.

[clarkliming]

We should discuss this more. I'm not saying I disagree but at the same time a degenerate tabletree object is really not very useful in practice. … On Mon, Mar 6, 2023, 6:54 AM Davide Garolini @.> wrote: So your suggestion is to have prune return an empty table instead of NULL. I think it makes sense. @gmbecker https://github.com/gmbecker were there some specific design principles behind this NULL output? — Reply to this email directly, view it on GitHub <#566 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG53MMTCRSUDSWVDAEH6XTW2X3DVANCNFSM6AAAAAAVRAPQZI . You are receiving this because you were mentioned.Message ID: @.>

I agree with you; but we should have consistent behavior, especially in the down stream, when we try to figure out if the table is empty, shall we cout the rows, or test if it is null? of course both can be handled but better to be consistent

[Melkiades]

As we want to proceed here? @clarkliming @gmbecker any pruning that produces an empty table should be sanitized or return NULL?