Skip to content

Adds support for tbl_split to table_with_settings#353

Open
averissimo wants to merge 10 commits into
mainfrom
999-tm_gtsummary
Open

Adds support for tbl_split to table_with_settings#353
averissimo wants to merge 10 commits into
mainfrom
999-tm_gtsummary

Conversation

@averissimo

Copy link
Copy Markdown
Contributor

Pull Request

Changes description

  • Add support for tbl_split from gtsummary
  • Allow for zip files to be used to download tables if they cannot be generated in single file

@averissimo averissimo changed the title feat: implements render of tbl_split Adds support for tbl_split to table_with_settings Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ------------------------------------------------------
R/basic_table_args.R             23       0  100.00%
R/draggable_buckets.R            87      12  86.21%   116-121, 151-156
R/get_dt_rows.R                  13       0  100.00%
R/ggplot2_args.R                 49       0  100.00%
R/nested_closeable_modal.R       20      20  0.00%    83-103
R/optionalInput.R               255      79  69.02%   190, 315-389, 402-409, 411-416, 434, 436, 501, 583-596
R/panel_group.R                  39      39  0.00%    50-136
R/plot_with_settings.R          310      16  94.84%   301-307, 329, 366, 375-376, 392, 580-581, 583, 585
R/standard_layout.R              52       4  92.31%   90-93
R/table_with_settings.R         231      53  77.06%   32-33, 51-55, 87-94, 115, 160-219
R/utils.R                         7       0  100.00%
R/verbatim_popup.R              104      35  66.35%   112-113, 115, 123-154
R/white_small_well.R              7       0  100.00%
TOTAL                          1197     258  78.45%

Diff against main

Filename                   Stmts    Miss  Cover
-----------------------  -------  ------  -------
R/table_with_settings.R      +32     +29  -10.88%
TOTAL                        +32     +29  -1.90%

Results for commit: 406165a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Unit Tests Summary

  1 files   16 suites   2m 9s ⏱️
172 tests 172 ✅ 0 💤 0 ❌
400 runs  400 ✅ 0 💤 0 ❌

Results for commit 406165a.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
plot_with_settings_ui 💔 $56.16$ $+2.57$ $0$ $0$ $0$ $0$

Results for commit 7eb42f4

♻️ This comment has been updated with latest results.

@llrs-roche llrs-roche left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving tables to a file can become complicated. Quite a lot of hours have been spent on how to do so that they comply with different policies and preferences.

However, when exporting to .csv there are unmatched quotes sometimes (from gtsummary::gt_summary() via tm_gt_template()):

Image

Comment thread R/table_with_settings.R
Comment thread R/table_with_settings.R Outdated
Comment on lines +195 to +196
tmp_files <- lapply(seq_along(x), function(i) {
tmp_file <- file.path(tmp_dir, paste0(base_name, "_", i, ext))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a bit silly but the splits are also used to remove some columns. It would be great if instead of a file6de06d6041ec_1.csv and a file6de06d6041ec_2.csv we would get a file name a bit more meaningful: the variable_level or such.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried by keeping original file name and adding the variable level when possible.

Check it out on 92270c2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't work, now files are file112361464645_1_1.csv or such. My guess is that the label is somehow dropped o missed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me check if there's unpushed code

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now with better naming:

<original_name>_<variable_level>_<index>

The index is just in case to avoid repetitions, and _<variable_level> is hidden if it's not a string

image

@llrs-roche llrs-roche self-assigned this Jul 14, 2026
@averissimo

Copy link
Copy Markdown
Contributor Author

@llrs-roche humm, I'm trying to check it out, thanks for pointing to that PR.

@llrs-roche

Copy link
Copy Markdown
Contributor

The problem with the quotes might have been how Excel dealt with it, but I'm not sure.

I checked again the pdfs and now they aren't displayed correctly: Long tables show a horizontal scroll bar that doesn't work:
Captura de pantalla 2026-07-14 154813

In any case I don't think we need to merge this for the demo (but the detail about sensible file names will be appreciated).

@averissimo

Copy link
Copy Markdown
Contributor Author

@llrs-roche All fixed and improved implementation of label usage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants