Discussion on colcounts accessor functions' scopes and groupings

Lgtm! Thanks Gabe for this update. I am wondering only if this is doing nothing on purpose:

lyt2 <- basic_table() %>%
  split_cols_by("ARM") %>%
  split_cols_by("SEX",
    split_fun = keep_split_levels(c("F", "M")),
    show_colcounts = TRUE
  ) %>%
  analyze("AGE")

tbl2 <- build_table(lyt2, ex_adsl)
tbl2
colcount_format(tbl2) <- "xx"
tbl2 # the same

So the issue here is with what that should do.
I think the most reasonable thing for that to do would be to reset all colcount formats throughout the column tree. The issue with that is that then colcount_format<- isn't really the setter that corresponds with colcount_format the getter anymore. They would have the same name but only somewhat corresponding behaviors.
Its probably still the right thing, but what are your thoughts? @Melkiades

To my mind, colcount_format<- and colcount_visibility<- should be able to set all to the same value when path = NULL or specific values when path is specified. Similarly, I understand why you added facet_colcounts<- and facet_colcounts_visibility<- but it is difficult to motivate to a generic user why you need another function to do a very similar thing. Personally, I would cover the 3 relevant things: formats, visibility, and values, with only one function each (in that case, for a facet is enough to use the node in the path)

There are a few things that I think make that a non-starter, though I agree it would be nice in principle.

First, we cannot do that for values, because assigning a vector of values onto the nodes of a tree structure isn't trivial or something the user is going to be able to easily understand.

In other words, if we have

> lyt <- basic_table() %>% split_cols_by("ARM") %>% split_cols_by("SEX") %>% analyze("AGE")
> tbl <- build_table(lyt, ex_adsl)
> tbl
                      A: Drug X                                  B: Placebo                               C: Combination             
         F       M       U     UNDIFFERENTIATED     F       M       U     UNDIFFERENTIATED     F       M       U     UNDIFFERENTIATED
—————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————
Mean   32.76   35.57   31.67        28.00         34.12   37.44   31.00          NA          35.20   35.38   35.25        45.00  

And we say

facet_colcounts(tbl, NULL) <- 1:15

where are each of those values going to be assigned? A: Drug X will likely get a 1 under it, but does the 2 go under B: Placebo" or under the FunderA: Drug X`? Its not clear to me and it would be extremely unclear to the user, I think.

Secondly, for backwards compatability we need col_counts<- with no path specification to only modify the leaf nodes. We might be able to hack that behavior in by differentiating between NULL and NA, but it would be ugly and complicate the code and the above issue would remain anyway

For visibility and format, the issue is different desired behavior. I am not convinced turning on visibility of all column counts for tables with non-trivial column structure would ever/even remotely often be what the user wants to do, they're likely to want to turn on and off specific (sets of) column counts. For format, though, the opposite is true, if they are ever in a position to want to modify the colcount format for an already built table, they very likely do want it to be changed across the board, and are quite unlikely to only want to change the format of specific column counts.

Originally posted by @gmbecker in https://github.com/insightsengineering/rtables/issues/876#issuecomment-2161421474

No files or tests are named. Start by reviewing the existing colcount_format, colcount_visibility, facet_colcounts, and col_counts accessors and their path handling. Resolve the intended behavior for formats, visibility, and values while preserving the stated backward-compatibility constraint; done means the semantics are agreed and covered by tests.