search

insightsengineering / tern

Table, Listings, and Graphs (TLG) library for common outputs used in clinical trials
https://insightsengineering.github.io/tern/
Other
77 stars 22 forks source link

fix decorate grob vp widths for titles and footers when right margin is present #1245

Closed Melkiades closed 6 months ago

Melkiades commented 6 months ago

1240

github-actions[bot] commented 6 months ago

CLA Assistant Lite bot ✅ All contributors have signed the CLA

Melkiades commented 6 months ago

I have read the CLA Document and I hereby sign the CLA

Melkiades commented 6 months ago

recheck

github-actions[bot] commented 6 months ago

Unit Tests Summary

    1 files     83 suites   1m 4s :stopwatch:   838 tests   826 :white_check_mark:  12 :zzz: 0 :x: 1 803 runs  1 130 :white_check_mark: 673 :zzz: 0 :x:

Results for commit edafc7aa.

:recycle: This comment has been updated with latest results.

github-actions[bot] commented 6 months ago

badge

Code Coverage Summary

Filename                                   Stmts    Miss  Cover    Missing
---------------------------------------  -------  ------  -------  ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/abnormal_by_baseline.R                      65       0  100.00%
R/abnormal_by_marked.R                        55       5  90.91%   78-82
R/abnormal_by_worst_grade_worsen.R           116       3  97.41%   242-244
R/abnormal_by_worst_grade.R                   60       0  100.00%
R/abnormal.R                                  43       0  100.00%
R/analyze_variables.R                        162       3  98.15%   488, 512, 628
R/analyze_vars_in_cols.R                     176      33  81.25%   179, 202-207, 222, 236-237, 245-253, 259-265, 344-350
R/bland_altman.R                              92       1  98.91%   43
R/combination_function.R                       9       0  100.00%
R/compare_variables.R                         84       5  94.05%   130-134, 246, 305
R/control_incidence_rate.R                    10       0  100.00%
R/control_logistic.R                           7       0  100.00%
R/control_step.R                              23       1  95.65%   58
R/control_survival.R                          15       0  100.00%
R/count_cumulative.R                          50       1  98.00%   67
R/count_missed_doses.R                        34       0  100.00%
R/count_occurrences_by_grade.R               113       5  95.58%   101, 151-153, 156
R/count_occurrences.R                        115       1  99.13%   108
R/count_patients_events_in_cols.R             67       1  98.51%   53
R/count_patients_with_event.R                 47       0  100.00%
R/count_patients_with_flags.R                 58       4  93.10%   56-57, 62-63
R/count_values.R                              27       0  100.00%
R/cox_regression_inter.R                     154       0  100.00%
R/cox_regression.R                           161       0  100.00%
R/coxph.R                                    167       7  95.81%   191-195, 238, 253, 261, 267-268
R/d_pkparam.R                                406       0  100.00%
R/decorate_grob.R                            113       0  100.00%
R/desctools_binom_diff.R                     621      64  89.69%   53, 88-89, 125-126, 129, 199, 223-232, 264, 266, 286, 290, 294, 298, 353, 356, 359, 362, 422, 430, 439, 444-447, 454, 457, 466, 469, 516-517, 519-520, 522-523, 525-526, 593, 604-616, 620, 663, 676, 680
R/df_explicit_na.R                            30       0  100.00%
R/estimate_multinomial_rsp.R                  50       1  98.00%   63
R/estimate_proportion.R                      205      12  94.15%   78-85, 89, 94, 315, 481, 587
R/fit_rsp_step.R                              36       0  100.00%
R/fit_survival_step.R                         36       0  100.00%
R/formatting_functions.R                     183       2  98.91%   143, 278
R/g_forest.R                                 585      60  89.74%   241, 253-256, 261-262, 276, 278, 288-291, 336-339, 346, 415, 502, 515, 519-520, 525-526, 539, 555, 602, 633, 708, 717, 723, 742, 797-817, 820, 831, 850, 905, 908, 1043-1048
R/g_ipp.R                                    133       0  100.00%
R/g_km.R                                     350      57  83.71%   286-289, 308-310, 364-367, 401, 429, 433-476, 483-487
R/g_lineplot.R                               238      28  88.24%   183, 196, 232, 272, 353-360, 383-384, 395-405, 497, 503, 505
R/g_step.R                                    68       1  98.53%   109
R/g_waterfall.R                               47       0  100.00%
R/h_adsl_adlb_merge_using_worst_flag.R        73       0  100.00%
R/h_biomarkers_subgroups.R                    45       0  100.00%
R/h_cox_regression.R                         110       0  100.00%
R/h_km.R                                     509      41  91.94%   137, 189-194, 287, 378, 380-381, 392-394, 413, 420-421, 423-425, 433-435, 460, 465-468, 651-654, 1108-1119
R/h_logistic_regression.R                    468       3  99.36%   203-204, 273
R/h_map_for_count_abnormal.R                  54       0  100.00%
R/h_pkparam_sort.R                            15       0  100.00%
R/h_response_biomarkers_subgroups.R           90      12  86.67%   50-55, 107-112
R/h_response_subgroups.R                     178      18  89.89%   257-270, 329-334
R/h_stack_by_baskets.R                        64       1  98.44%   89
R/h_step.R                                   180       0  100.00%
R/h_survival_biomarkers_subgroups.R           88       6  93.18%   111-116
R/h_survival_duration_subgroups.R            207      18  91.30%   259-271, 336-341
R/imputation_rule.R                           17       2  88.24%   54-55
R/incidence_rate.R                            96       7  92.71%   44-51
R/logistic_regression.R                      102       0  100.00%
R/missing_data.R                              21       3  85.71%   32, 66, 76
R/odds_ratio.R                               109       0  100.00%
R/prop_diff_test.R                            91       0  100.00%
R/prop_diff.R                                265      16  93.96%   62-65, 97, 282-289, 432, 492, 597
R/prune_occurrences.R                         57      10  82.46%   138-142, 188-192
R/response_biomarkers_subgroups.R             68       6  91.18%   189-194
R/response_subgroups.R                       192      10  94.79%   95-100, 276, 324-326
R/riskdiff.R                                  59       7  88.14%   102-105, 114, 124-125
R/rtables_access.R                            38       4  89.47%   159-162
R/score_occurrences.R                         20       1  95.00%   124
R/split_cols_by_groups.R                      49       0  100.00%
R/stat.R                                      59       3  94.92%   73-74, 129
R/summarize_ancova.R                         106       2  98.11%   174, 179
R/summarize_change.R                          30       0  100.00%
R/summarize_colvars.R                         10       0  100.00%
R/summarize_coxreg.R                         172       2  98.84%   203, 430
R/summarize_glm_count.R                      195      27  86.15%   206, 224-256, 301-302
R/summarize_num_patients.R                    93       5  94.62%   108-110, 244-245
R/summarize_patients_exposure_in_cols.R       96       1  98.96%   42
R/survival_biomarkers_subgroups.R             78       6  92.31%   113-118
R/survival_coxph_pairwise.R                   79      11  86.08%   45-46, 58-66
R/survival_duration_subgroups.R              197       6  96.95%   119-124
R/survival_time.R                             79       0  100.00%
R/survival_timepoint.R                       113       7  93.81%   120-126
R/utils_checkmate.R                           68       0  100.00%
R/utils_default_stats_formats_labels.R       116       1  99.14%   72
R/utils_factor.R                             109       2  98.17%   84, 302
R/utils_ggplot.R                             110       0  100.00%
R/utils_grid.R                               126       5  96.03%   164, 279-286
R/utils_rtables.R                            100       9  91.00%   39, 46, 51, 58-62, 403-404
R/utils_split_funs.R                          52       2  96.15%   82, 94
R/utils.R                                    141       7  95.04%   118, 121, 124, 128, 137-138, 332
TOTAL                                      10405     556  94.66%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/decorate_grob.R       +1       0  +100.00%
TOTAL                   +1       0  +0.00%

Results for commit: edafc7aac9cfc5d530becd32a5d056c9b3700e4a

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

Melkiades commented 6 months ago

Can someone please help me review this? @ayogasekaram @edelarua Thanks!!

pawelru commented 6 months ago

Hey Davide. I tried with the example from the issue but I still can't make it work properly. width_titles = grid::unit(1, "npc") should indicate full width without margins but apparently it's not like that...

image

Tested in RStudio for now. For some reason I struggle to test in VSCode but I will look into this next.

pawelru commented 6 months ago

On VSCode it's ok... Let me dig more into this: image

Melkiades commented 6 months ago

@pawelru rstudio does not work superwell with viewport as far as I could see. Everything works for base R on my machine. Have not tried VS but my output in rstudio is identical to vs

pawelru commented 6 months ago

You are right. Base R should be the main testing environment. Tested it there and it's good. Also tested your feature branch versus the main and I can notice that the issue is fixed now. I think it's good to go. One question though. Is this already covered by any test? I can see edits in one test but it's about margins - not the width of tittle and/or footnote. If not - let's add a new one.

Melkiades commented 6 months ago

You are right. Base R should be the main testing environment. Tested it there and it's good. Also tested your feature branch versus the main and I can notice that the issue is fixed now. I think it's good to go. One question though. Is this already covered by any test? I can see edits in one test but it's about margins - not the width of tittle and/or footnote. If not - let's add a new one.

It is a valid point; I could not think of a proper test for it. I can use the width of the viewport as a test. It is what needs to be respected for the wrapping at the end.

Btw the wrapping core is not keeping spaces and manual \n as far as I can see. We might want to use the one I wrote for {formatters} that keeps those. What do you think?

pawelru commented 6 months ago

Btw the wrapping core is not keeping spaces and manual \n as far as I can see. We might want to use the one I wrote for {formatters} that keeps those. What do you think?

This I don't know to be honest. I'm afraid that I'm not that deep in this to make a statement.

I'm going to approve as I can see that the issue had been resolved. Please make a call about the test. If it can't be done then I guess we need to live with that.

Melkiades commented 6 months ago

Btw the wrapping core is not keeping spaces and manual \n as far as I can see. We might want to use the one I wrote for {formatters} that keeps those. What do you think?

This I don't know to be honest. I'm afraid that I'm not that deep in this to make a statement.

I'm going to approve as I can see that the issue had been resolved. Please make a call about the test. If it can't be done then I guess we need to live with that.

I added and closed an issue to track down my research. Testing for this issue can be a bit difficult considering the inconsistencies between different viewport page sizes' conversions