support displaying column counts at higher levels in column structure #856
Conversation
gmbecker
requested review from
edelarua,
Melkiades and
ayogasekaram
as code owners
| ## likely unneeded now because it happens in splitvec_to_coltree | ||
| ## which is called during coltree construction above | ||
| ## TODO remove me |
There was a problem hiding this comment.
if it is not run at all anymore we can already remove it
| #' Get or set column count for a facet in column space | ||
| #' | ||
| #' @inheritParams gen_args | ||
| #' @param path character. This path must end on a |
There was a problem hiding this comment.
| #' @param path character. This path must end on a | |
| #' @param path (`string`)\cr this path must end on a |
There was a problem hiding this comment.
This is incorrect. A string, in as much as it exists at all (which, again, in R it doesnt), has length one, whereas a path is a character vector of any length.
There was a problem hiding this comment.
I will admit I forgot to put the weird new stuff around it though...
|
Also, I don't really have any idea whats going on with the spell checker, it appears to be flagging files that this PR doesn't change (e.g., tt_as_flextable.Rd) I can't easily use pre-commit in my fork because the hooks have been updated from |
Ok investigatiNg this. |
Shot in the dark, if it is running spell check on the full diffs instead of the additions, it could be catching things that were removed. That could explain why it always complains so much and about seemingly random things, but it wouldn't explain complaining about files I didn't touch at all, so I dunno |
There was a problem hiding this comment.
@gmbecker @shajoezhu I think before this one we should merge the other outstanding PRs from Gabe. As soon as those are in we can review this PR more in detail. Should we start to get #819 passing the tests?
| #' Set visibility of column counts for a group of sibling facets | ||
| #' | ||
| #' @inheritParams gen_args | ||
| #' @param path character. The path *to the parent of the |
There was a problem hiding this comment.
| #' @param path character. The path *to the parent of the | |
| #' @param path (`character`)\cr the path *to the parent of the |
| #' desired siblings*. The last element in the path should | ||
| #' be a split name. |
There was a problem hiding this comment.
| #' desired siblings*. The last element in the path should | |
| #' be a split name. | |
| #' desired siblings*. The last element in the path should | |
| #' be a split name. |
| #' Get or set column count for a facet in column space | ||
| #' | ||
| #' @inheritParams gen_args | ||
| #' @param path character. This path must end on a |
There was a problem hiding this comment.
| #' @param path character. This path must end on a | |
| #' @param path (`character`)\cr this path must end on a |
|
Converted to draft because it turns out this requires a large rework how cbinding works befcause the current way of cbinding does not result in column structure where all elements are path-addressable. I have this working in the combined branch we are working off of at J&J but I haven't cherrypicked the changes to put them into this PR yet. I will do so (or it won't matter if we get the truetype stuff merged in first) |
#819 is subsumed by the truetype font PR, which was derailed by the large scale doc changes causing tons of conflicts. The corresponding formatters PR exists, and I'll work on getting the rtables one up, but that is the one we should use |
|
Closing this PR and will open another one after #865 is merged in (I have a bundled branch we are doing our work with that has both features so I'll merge that into a new branch after the first PR settles) |
clean lint, includes tests, doesn't include updates to vignette(s)