pharmaverse / pharmaverse/rtables

col_paths is overly expensive

Open
#1,035 1 comment 1 reaction 1 assignee View on GitHub

@gmbecker is already working on this.

Since May 19, 2026.

collaboration enhancement
Dominant language
R
Stars
260
Forks
53
PR merge metrics
No merged PRs in 30d

Description

Currently col_paths is implemented as

function (x) 
{
    if (!is(coltree(x), "LayoutColTree")) {
        stop("I don't know how to extract the column paths from an object of class ", 
            class(x))
    }
    make_col_df(x, visible_only = TRUE)$path
}

The problem is make_col_df does a ton of other things unrelated to column paths. Combine this with the fact that pruning or scoring functions may need to call col_paths for every row of a table (if implemented naively) and this gives rise to a situation where for large tables we have seen repeated col_paths calls take up to 50% of the total pruning/sorting time, when each call in those contexts is guaranteed to return the same set of paths making that time entirely wasted.

I propose we extend the InstantiatedColumnInfo class to cache its set of column paths the way it already does for column subset expressions. This would make repeated col_paths calls acceptable as each one is effectively free.

In fact, the result of make_col_df doesn't depend on font the way make_row_df does, so I think we could consider caching the full result of make_col_df rather than just the col_paths...

Contributor guide

Open the contributing guide

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.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.