Merck / Merck/simtrial

Simplify lt.R using lt predicate selectors and spanner auto-reorder (next lt release)

Open
#373 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
R
Stars
39
Forks
14
Avg merge
1d 8h
Merged PRs (30d)
7

Description

Context

PR #372 replaced gt with lt for summary tables. To work with the current CRAN lt (0.4), R/lt.R had to work around two lt limitations:

  1. No pattern-based column selection — spanner/label columns are enumerated with grep("_time$", names(x), value = TRUE) etc.
  2. Spanner columns had to be listed in visual (post-lt_move) order — lt matched a spanner to the visual position of its first column and spanned the next length(columns) cells, so a mismatched order silently dropped later spanners (this caused the "Probability of crossing efficacy bounds under H1" spanner to go missing; fixed in 76a63ca by listing columns in explicit asy-before-sim order).

Both limitations are addressed in the next lt version (merged upstream in yihui/lt#5 plus follow-up yihui/lt@50c09bc):

  • Predicate formulas: columns = ~ endsWith(., "_time"), ~ grepl("_prob$", .), ~ . (all columns), etc.
  • lt_label() accepts a single named list/vector of labels (no more do.call() splice).
  • Spanner columns auto-reordered to body order at render, so their listed order no longer matters (predicate selectors are safe for spanners too).

Task (do this once lt >= that version is on CRAN)

In R/lt.R:

  • Replace the grep(..., names(x), value = TRUE) spanner enumerations (and the explicit c("asy_time", "sim_time") ordering added in 76a63ca) with predicate selectors, e.g. lt_spanner(Time ~ endsWith(., "_time")). Spanner auto-reorder means order no longer needs hand-managing.
  • Replace the do.call(lt::lt_label, c(list(ans), labels)) splice with ans |> lt::lt_label(labels) (single named vector). See the review suggestion on PR #372.

In vignettes/discrepancy-between-simtrial-and-survival.Rmd:

  • Replace lt_format(columns = names(fr)[sapply(fr, is.numeric)], ...) with the everything() equivalent columns = ~ ..

In DESCRIPTION:

  • Bump the lt Imports version constraint to the release that ships these features (lt (>= X.Y.Z)).

References

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

First verify that the required lt release is available on CRAN, then read R/lt.R, vignettes/discrepancy-between-simtrial-and-survival.Rmd, and DESCRIPTION. Replace the listed selectors and label call, update the lt constraint, and confirm all task checkboxes are satisfied without the explicit spanner ordering workaround.

Written by the indexing model from the issue text.

Assessment

Tech stack
r
Domain
data-visualization
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.