Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor g_km #1210

Merged
merged 44 commits into from
Mar 21, 2024
Merged

Refactor g_km #1210

merged 44 commits into from
Mar 21, 2024

Conversation

edelarua
Copy link
Contributor

@edelarua edelarua commented Mar 15, 2024

Pull Request

Fixes #1167

I'm not sure if the grob-related functions are used in other places - if so we may not be able to deprecate them currently. If/when we remove all of these functions we should be able to remove the dependencies on packages grid, gridExtra, and gtable.

@edelarua edelarua added the sme label Mar 15, 2024
Copy link
Contributor

github-actions bot commented Mar 15, 2024

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%   240-242
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%   487, 511, 627
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%   37
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, 239, 254, 262, 268-269
R/d_pkparam.R                                406       0  100.00%
R/decorate_grob.R                            193      40  79.27%   241-272, 338-340, 351, 372-409
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, 482, 588
R/fit_rsp_step.R                              36       0  100.00%
R/fit_survival_step.R                         36       0  100.00%
R/formatting_functions.R                     181       2  98.90%   145, 280
R/g_forest.R                                 585     429  26.67%   183-186, 189-192, 195-201, 204-207, 210-213, 240, 252-255, 260-261, 275, 277, 287-290, 335-338, 345, 414, 493-1042
R/g_km.R                                     295      57  80.68%   290-293, 312-314, 368-371, 405, 433, 437-480, 487-491
R/g_lineplot.R                               209      34  83.73%   173, 186, 215, 241-244, 320-327, 345-346, 352-362, 454, 460, 462, 504-505, 509-510
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     352  30.84%   138, 190-195, 288, 369-1006, 1109-1120
R/h_logistic_regression.R                    468       3  99.36%   206-207, 276
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/individual_patient_plot.R                  133       0  100.00%
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                         104       2  98.08%   172, 177
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             70       6  91.43%   112-117
R/survival_coxph_pairwise.R                   79      11  86.08%   45-46, 58-66
R/survival_duration_subgroups.R              191       6  96.86%   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%   161, 276-283
R/utils_rtables.R                            100       9  91.00%   39, 46, 51, 58-62, 403-404
R/utils_split_funs.R                          52       2  96.15%   81, 93
R/utils.R                                    141      10  92.91%   92, 94, 98, 118, 121, 124, 128, 137-138, 332
TOTAL                                      10383    1285  87.62%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/decorate_grob.R      +20       0  +2.40%
R/g_forest.R             0      +1  -0.17%
R/g_km.R              +295     +57  +80.68%
R/h_km.R              +509    +352  +30.84%
R/utils_ggplot.R       +38       0  +100.00%
R/utils_grid.R         +15       0  +0.54%
TOTAL                 +877    +410  -3.05%

Results for commit: 69e1632

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Mar 15, 2024

Unit Tests Summary

    1 files     83 suites   1m 7s ⏱️
  822 tests   790 ✅  32 💤 0 ❌
1 738 runs  1 071 ✅ 667 💤 0 ❌

Results for commit 69e1632.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Mar 15, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
g_km 💔 $0.08$ $+4.81$ $+3$ $+3$ $0$ $0$
h_km 👶 $+0.40$ $+28$ $+13$ $0$ $0$
kaplan_meier_plot 💀 $1.90$ $-1.90$ $-27$ $-11$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
g_km 👶 $+0.49$ g_km_as_list_argument_works
g_km 👶 $+0.65$ g_km_works_with_custom_arguments
g_km 👶 $+0.62$ g_km_works_with_title_footnotes_and_annotation
h_km 👶 $+0.02$ control_coxph_annot_works_with_default_settings
h_km 👶 $+0.02$ control_surv_med_annot_works_with_default_settings
h_km 👶 $+0.02$ h_data_plot_adds_rows_that_have_time_0_and_estimate_1
h_km 👶 $+0.03$ h_data_plot_respects_the_ordering_of_the_arm_variable_factor_levels
h_km 👶 $+0.02$ h_data_plot_works_as_expected
h_km 👶 $+0.08$ h_grob_coxph_returns_error_when_only_one_arm
h_km 👶 $+0.03$ h_tbl_coxph_pairwise_estimates_HR_CI_and_pvalue
h_km 👶 $+0.02$ h_tbl_median_surv_estimates_median_survival_time_with_CI
h_km 👶 $+0.02$ h_xticks_returns_error_when_xticks_non_numeric
h_km 👶 $+0.02$ h_xticks_works_with_default_settings
h_km 👶 $+0.01$ h_xticks_works_with_max_time_only
h_km 👶 $+0.02$ h_xticks_works_with_xticks_number
h_km 👶 $+0.03$ h_xticks_works_with_xticks_number_when_max_time_is_not_NULL
h_km 👶 $+0.02$ h_xticks_works_with_xticks_numeric
h_km 👶 $+0.03$ h_xticks_works_with_xticks_numeric_when_max_time_is_not_NULL
kaplan_meier_plot 💀 $0.67$ $-0.67$ g_km_works_with_custom_settings
kaplan_meier_plot 💀 $0.42$ $-0.42$ g_km_works_with_default_settings
kaplan_meier_plot 💀 $0.48$ $-0.48$ g_km_works_with_title_footnotes_and_annotation
kaplan_meier_plot 💀 $0.03$ $-0.03$ h_data_plot_adds_rows_that_have_time_0_and_estimate_1
kaplan_meier_plot 💀 $0.04$ $-0.04$ h_data_plot_respects_the_ordering_of_the_arm_variable_factor_levels
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_data_plot_works_as_expected
kaplan_meier_plot 💀 $0.05$ $-0.05$ h_grob_coxph_returns_error_when_only_one_arm
kaplan_meier_plot 💀 $0.04$ $-0.04$ h_tbl_coxph_pairwise_estimates_HR_CI_and_pvalue
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_tbl_median_surv_estimates_median_survival_time_with_CI
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_xticks_returns_error_when_xticks_non_numeric
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_xticks_works_with_default_settings
kaplan_meier_plot 💀 $0.01$ $-0.01$ h_xticks_works_with_max_time_only
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_xticks_works_with_xticks_number
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_xticks_works_with_xticks_number_when_max_time_is_not_NULL
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_xticks_works_with_xticks_numeric
kaplan_meier_plot 💀 $0.02$ $-0.02$ h_xticks_works_with_xticks_numeric_when_max_time_is_not_NULL
utils_ggplot 👶 $+0.03$ df2gg_works_as_expected

Results for commit f817de3

♻️ This comment has been updated with latest results.

@Melkiades
Copy link
Contributor

Pull Request

Fixes #1167

I'm not sure if the grob-related functions are used in other places - if so we may not be able to deprecate them currently. If/when we remove all of these functions we should be able to remove the dependencies on packages grid, gridExtra, and gtable.

Talking with Kartik yesterday, I saw that grob is used to produce reports in teal.report. It may be useful then to move towards removing this dep also there, as it is not maintained (right?). @kartikeyakirar fyi

DESCRIPTION Outdated Show resolved Hide resolved
tests/testthat/test-h_km.R Outdated Show resolved Hide resolved
R/g_km.R Outdated Show resolved Hide resolved
Copy link
Contributor

@shajoezhu shajoezhu left a comment

Choose a reason for hiding this comment

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

looks really great! thanks @edelarua

NEWS.md Show resolved Hide resolved
@edelarua edelarua merged commit 036eb92 into main Mar 21, 2024
25 checks passed
@edelarua edelarua deleted the 1167_refactor_g_km@main branch March 21, 2024 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate feasibility of exclusively using ggplot2 for all plots - part II (g_km)
6 participants