You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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
And we say
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 underB: Placebo" or under the
Funder
A: 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 anywayFor 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 #876 (comment)
The text was updated successfully, but these errors were encountered: