Skip to content

Batch 1#1403

Merged
Melkiades merged 16 commits intomainfrom
1381_batch_1_no_riskdiff@main
Mar 10, 2025
Merged

Batch 1#1403
Melkiades merged 16 commits intomainfrom
1381_batch_1_no_riskdiff@main

Conversation

@Melkiades
Copy link
Contributor

@Melkiades Melkiades commented Feb 28, 2025

Fixes part of #1381 and #1249 and #1390

@Melkiades Melkiades added the sme label Feb 28, 2025
This was linked to issues Feb 28, 2025
export tern:::as_factor_keep_attributes #1249
Closed
R CMD CHECK note #1390
Closed
Melkiades
Melkiades commented Feb 28, 2025
View reviewed changes
Copy link
Contributor Author

@Melkiades Melkiades left a comment

Choose a reason for hiding this comment

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

@edelarua I think we can merge this, your PR and then refactor riskdiff if we deem it necessary. I think the behavior there is strange. I would prefer adding a subfunction to a_* functions to deal with that cases. I manage to get a weird hybrid on the other PR but it is better to have a clean slate with most recent updates

@github-actions
Copy link
Contributor

github-actions bot commented Feb 28, 2025 •
edited
Loading

Unit Tests Summary

    1 files     85 suites   1m 17s ⏱️
  878 tests   868 ✅  10 💤 0 ❌
1 907 runs  1 210 ✅ 697 💤 0 ❌

Results for commit fb453fb.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 28, 2025 •
edited
Loading

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
count_cumulative 👶 $+0.05$ Testing_label_behavior_when_s__forecasts_label_attributes
count_missed_doses 💀 $0.02$ $-0.02$ s_count_nonmissing_also_works_with_character_input
count_missed_doses 💀 $0.02$ $-0.02$ s_count_nonmissing_works_with_numeric_input

Results for commit 82ad2e8

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2025 •
edited
Loading

badge

Code Coverage Summary

Filename                                   Stmts    Miss  Cover    Missing
---------------------------------------  -------  ------  -------  ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/abnormal_by_baseline.R                     101       3  97.03%   242, 244-245
R/abnormal_by_marked.R                        88       8  90.91%   94-98, 281, 283-284
R/abnormal_by_worst_grade.R                   94       3  96.81%   215, 217-218
R/abnormal_lab_worsen_by_baseline.R          159      10  93.71%   205-208, 213, 215-216, 459-461
R/abnormal.R                                  78       2  97.44%   222, 224
R/analyze_variables.R                        287       5  98.26%   587-590, 778
R/analyze_vars_in_cols.R                     178      14  92.13%   178, 221, 235-236, 238, 246-254
R/bland_altman.R                              92       1  98.91%   46
R/combination_function.R                       9       0  100.00%
R/compare_variables.R                         35       0  100.00%
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                         115       4  96.52%   74, 270-271, 273
R/count_missed_doses.R                        89       4  95.51%   206-209
R/count_occurrences_by_grade.R               163       5  96.93%   178, 386, 388, 465, 467
R/count_occurrences.R                        131       8  93.89%   119, 262-264, 330-333
R/count_patients_events_in_cols.R             67       1  98.51%   60
R/count_patients_with_event.R                 73       2  97.26%   220, 223
R/count_patients_with_flags.R                 93       2  97.85%   234, 236
R/count_values.R                              61       2  96.72%   193, 196
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                  86       4  95.35%   65, 212, 214-215
R/estimate_proportion.R                      229       7  96.94%   84, 95, 236, 238-239, 370, 536
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%   141, 276
R/g_forest.R                                 585      60  89.74%   240, 252-255, 260-261, 275, 277, 287-290, 335-338, 345, 414, 501, 514, 518-519, 524-525, 538, 554, 601, 630, 705, 714, 720, 739, 794-814, 817, 828, 847, 902, 905, 1040-1045
R/g_ipp.R                                    133       0  100.00%
R/g_km.R                                     350      57  83.71%   285-288, 307-309, 363-366, 400, 428, 432-475, 482-486
R/g_lineplot.R                               260      22  91.54%   204, 378-385, 424-434, 543, 551
R/g_step.R                                    68       1  98.53%   108
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                    91      23  74.73%   40-42, 84-103
R/h_cox_regression.R                         110       0  100.00%
R/h_incidence_rate.R                          45       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           77      12  84.42%   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           73       6  91.78%   111-116
R/h_survival_duration_subgroups.R            207      18  91.30%   259-271, 336-341
R/imputation_rule.R                           17       0  100.00%
R/incidence_rate.R                           103       7  93.20%   68-73, 242
R/logistic_regression.R                      102       0  100.00%
R/missing_data.R                              21       3  85.71%   32, 66, 76
R/odds_ratio.R                               157       4  97.45%   270-273
R/prop_diff_test.R                           144       2  98.61%   230, 232
R/prop_diff.R                                318      17  94.65%   71-74, 106, 299, 301, 373-380, 523, 688
R/prune_occurrences.R                         57       0  100.00%
R/response_biomarkers_subgroups.R            124      10  91.94%   88-91, 270-275
R/response_subgroups.R                       247      16  93.52%   100-105, 271-275, 280, 282-283, 310-311
R/riskdiff.R                                  65       4  93.85%   94-97
R/rtables_access.R                            38       0  100.00%
R/score_occurrences.R                         20       1  95.00%   124
R/split_cols_by_groups.R                      49       0  100.00%
R/stat.R                                      59       0  100.00%
R/summarize_ancova.R                         142       2  98.59%   316-317
R/summarize_change.R                          71       3  95.77%   175, 177-178
R/summarize_colvars.R                         13       1  92.31%   75
R/summarize_coxreg.R                         172       0  100.00%
R/summarize_glm_count.R                      269      10  96.28%   129-130, 202-203, 459-463, 596
R/summarize_num_patients.R                   115       9  92.17%   122-124, 244-245, 247, 329-330, 332
R/summarize_patients_exposure_in_cols.R      155       7  95.48%   58, 232-233, 237, 357-358, 362
R/survival_biomarkers_subgroups.R            136      10  92.65%   117-122, 228-231
R/survival_coxph_pairwise.R                  124       5  95.97%   52-53, 248, 250-251
R/survival_duration_subgroups.R              245      15  93.88%   124-129, 268-273, 286, 288-289
R/survival_time.R                            120       1  99.17%   251
R/survival_timepoint.R                       153       2  98.69%   302, 304
R/utils_checkmate.R                           68       0  100.00%
R/utils_default_stats_formats_labels.R       186       0  100.00%
R/utils_factor.R                              87       1  98.85%   99
R/utils_ggplot.R                             110       0  100.00%
R/utils_grid.R                               126       5  96.03%   164, 279-286
R/utils_rtables.R                            125       9  92.80%   39, 46, 405-406, 528-532
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                                      11908     574  95.18%

Diff against main

Filename                                  Stmts    Miss  Cover
--------------------------------------  -------  ------  --------
R/count_cumulative.R                        +56      +3  -1.78%
R/count_missed_doses.R                      +53      +4  -4.49%
R/prop_diff_test.R                          +53      +2  -1.39%
R/prop_diff.R                               +53      +2  +0.31%
R/utils_default_stats_formats_labels.R       +8       0  +100.00%
TOTAL                                      +223     +11  -0.00%

Results for commit: 60bc5c0

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@Melkiades Melkiades requested a review from edelarua March 3, 2025 17:01
@Melkiades Melkiades changed the title Batch 1 initial part Batch 1 Mar 4, 2025
edelarua
edelarua suggested changes Mar 5, 2025
View reviewed changes
Copy link
Contributor

@edelarua edelarua left a comment

Choose a reason for hiding this comment

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

Hi @Melkiades,

Great work!! I just have a few comments/suggestions but otherwise everything looks good :)

Melkiades
Melkiades commented Mar 7, 2025
View reviewed changes
Melkiades
Melkiades commented Mar 7, 2025
View reviewed changes
Melkiades
Melkiades commented Mar 7, 2025
View reviewed changes
edelarua
edelarua approved these changes Mar 7, 2025
View reviewed changes
Copy link
Contributor

@edelarua edelarua left a comment

Choose a reason for hiding this comment

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

This looks good to me! Just one typo I saw but otherwise whenever you're ready you can test on scda.test & then merge.

Melkiades and others added 2 commits March 10, 2025 16:22
@Melkiades @edelarua
Apply suggestions from code review
60bc5c0 
Co-authored-by: Emily de la Rua <emily.de_la_rua@contractors.roche.com>
Signed-off-by: Davide Garolini <dgarolini@gmail.com>
@Melkiades
fix
fb453fb
@Melkiades Melkiades enabled auto-merge (squash) March 10, 2025 15:41
@Melkiades Melkiades merged commit 2f31365 into main Mar 10, 2025
27 checks passed
@Melkiades Melkiades deleted the 1381_batch_1_no_riskdiff@main branch March 10, 2025 15:50
@github-actions github-actions bot locked and limited conversation to collaborators Mar 10, 2025
@edelarua edelarua linked an issue Mar 10, 2025 that may be closed by this pull request
Retire make_afun() - Batch 1 #1381
Closed
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@shajoezhu shajoezhu shajoezhu left review comments

+1 more reviewer

@edelarua edelarua edelarua approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

sme

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

R CMD CHECK note Retire make_afun() - Batch 1 export tern:::as_factor_keep_attributes

3 participants

@Melkiades @shajoezhu @edelarua