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.

Sign up for GitHub

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

Jump to bottom

Add character method to h_coxreg_inter_effect #974

Merged
merged 19 commits into from
Jun 22, 2023
Merged

Add character method to h_coxreg_inter_effect #974

merged 19 commits into from
Jun 22, 2023

Conversation

edelarua
Copy link
Contributor

Closes #968

@edelarua edelarua added the sme label Jun 19, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 19, 2023 •
edited
Loading

badge

Code Coverage Summary

Filename                                   Stmts    Miss  Cover    Missing
---------------------------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/abnormal_by_baseline.R                      62       0  100.00%
R/abnormal_by_marked.R                        52       5  90.38%   115-119
R/abnormal_by_worst_grade_worsen.R           113       3  97.35%   233-235
R/abnormal_by_worst_grade.R                   37       0  100.00%
R/abnormal.R                                  40       0  100.00%
R/analyze_vars_in_cols.R                     113      23  79.65%   164, 188-193, 206, 219-225, 268-274, 305
R/combination_function.R                       9       0  100.00%
R/compare_variables.R                        139       5  96.40%   126-127, 137, 241, 259
R/control_incidence_rate.R                    20       8  60.00%   32-35, 38-41
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                          47       1  97.87%   63
R/count_missed_doses.R                        31       0  100.00%
R/count_occurrences_by_grade.R                84       6  92.86%   156-158, 161, 176-177
R/count_occurrences.R                         61       1  98.36%   92
R/count_patients_events_in_cols.R             67       1  98.51%   62
R/count_patients_with_event.R                 33       0  100.00%
R/count_patients_with_flags.R                 39       0  100.00%
R/count_values.R                              24       0  100.00%
R/cox_regression_inter.R                     154       0  100.00%
R/cox_regression.R                           161       0  100.00%
R/coxph.R                                    169       9  94.67%   19-20, 213-217, 261, 276, 284, 290-291
R/d_pkparam.R                                406       0  100.00%
R/decorate_grob.R                            169      40  76.33%   232-263, 323-325, 332, 353-390
R/desctools_binom_diff.R                     663      66  90.05%   55, 90-91, 131-132, 135, 214, 240-249, 288, 290, 310, 314, 318, 322, 378, 381, 384, 387, 448, 456, 468-469, 475-478, 486, 489, 498, 501, 549-550, 552-553, 555-556, 558-559, 629, 641-654, 659, 706, 719, 723
R/df_explicit_na.R                            30       0  100.00%
R/estimate_multinomial_rsp.R                  47       1  97.87%   60
R/estimate_proportion.R                      198      11  94.44%   75-82, 86, 91, 460, 565
R/fit_rsp_step.R                              36       0  100.00%
R/fit_survival_step.R                         36       0  100.00%
R/formatting_functions.R                     115       3  97.39%   107, 145, 155
R/g_forest.R                                 437      23  94.74%   197, 248-249, 316, 333-334, 339-340, 353, 369, 416, 447, 523, 532, 613-617, 627, 702, 705, 829
R/g_lineplot.R                               199      29  85.43%   160, 173, 201, 227-230, 307-314, 332-333, 339-349, 441, 449
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        74       0  100.00%
R/h_biomarkers_subgroups.R                    40       0  100.00%
R/h_cox_regression.R                         110       0  100.00%
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           75       0  100.00%
R/h_response_subgroups.R                     171      12  92.98%   257-270
R/h_stack_by_baskets.R                        65       1  98.46%   91
R/h_step.R                                   180       0  100.00%
R/h_survival_biomarkers_subgroups.R           79       0  100.00%
R/h_survival_duration_subgroups.R            200      12  94.00%   259-271
R/incidence_rate.R                            93       7  92.47%   55-62
R/individual_patient_plot.R                  133       0  100.00%
R/kaplan_meier_plot.R                        630      61  90.32%   260-295, 304-308, 508, 680-682, 690-692, 717, 724-725, 897, 1086, 1403-1414
R/logistic_regression.R                      101       0  100.00%
R/missing_data.R                              21       3  85.71%   32, 66, 76
R/odds_ratio.R                               106       0  100.00%
R/prop_diff_test.R                            88       0  100.00%
R/prop_diff.R                                260      16  93.85%   72-75, 107, 267-274, 413, 473, 578
R/prune_occurrences.R                         57      10  82.46%   138-142, 188-192
R/response_biomarkers_subgroups.R             60       0  100.00%
R/response_subgroups.R                       165       4  97.58%   273, 315-317
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                                      47       3  93.62%   73-74, 129
R/summarize_ancova.R                          95       1  98.95%   180
R/summarize_change.R                          27       0  100.00%
R/summarize_colvars.R                          6       0  100.00%
R/summarize_coxreg.R                         156       2  98.72%   198, 410
R/summarize_glm_count.R                      164       4  97.56%   159, 164, 208, 261
R/summarize_num_patients.R                    74       9  87.84%   103-105, 150-151, 217-222
R/summarize_patients_exposure_in_cols.R       97       1  98.97%   56
R/summarize_variables.R                      220       2  99.09%   266, 492
R/survival_biomarkers_subgroups.R             60       0  100.00%
R/survival_coxph_pairwise.R                   73       9  87.67%   59-67
R/survival_duration_subgroups.R              172       0  100.00%
R/survival_time.R                             47       0  100.00%
R/survival_timepoint.R                       116       7  93.97%   126-132
R/utils_checkmate.R                           68       0  100.00%
R/utils_factor.R                              87       1  98.85%   84
R/utils_grid.R                               111       5  95.50%   149, 258-264
R/utils_rtables.R                             86       7  91.86%   24, 31-35, 346-347
R/utils.R                                    137      10  92.70%   92, 94, 98, 118, 121, 124, 128, 137-138, 311
TOTAL                                       9146     432  95.28%

Diff against main

Filename                    Stmts    Miss  Cover
------------------------  -------  ------  --------
R/cox_regression_inter.R      +12       0  +100.00%
TOTAL                         +12       0  +0.01%

Results for commit: 5f3796a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Jun 19, 2023 •
edited
Loading

Unit Tests Summary

       1 files    78 suites   1m 31s ⏱️
   738 tests 738 ✔️     0 💤 0 ❌
1 567 runs  984 ✔️ 583 💤 0 ❌

Results for commit 7678607.

♻️ This comment has been updated with latest results.

@shajoezhu
Copy link
Contributor

Thanks @edelarua , blocking the merge until the 0.8.3 branch is merged

@shajoezhu shajoezhu marked this pull request as draft June 19, 2023 22:23
@Melkiades Melkiades marked this pull request as ready for review June 20, 2023 07:45
shajoezhu
shajoezhu reviewed Jun 20, 2023
View reviewed changes
R/cox_regression_inter.R Outdated Show resolved Hide resolved
shajoezhu
shajoezhu requested changes Jun 20, 2023
View reviewed changes
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.

Thanks a lot Emily!

Acutally, I was wondering if you could use liming's example in #968 (comment)

and introduce two similar test cases here. Thanks! with two covariate variables

@edelarua
Copy link
Contributor Author

Thanks a lot Emily!

Acutally, I was wondering if you could use liming's example in #968 (comment)

and introduce two similar test cases here. Thanks! with two covariate variables

I've added in a few additional tests for these cases corresponding to Liming's examples.

@edelarua edelarua requested a review from shajoezhu June 21, 2023 01:55
@clarkliming
Copy link
Contributor

minor comment:

tern/R/cox_regression_inter.R

Line 146 in 605de00

level = as.character(levels(data[[covar]])),

levels() should already return a character result

@shajoezhu shajoezhu requested a review from clarkliming June 21, 2023 07:12
clarkliming
clarkliming approved these changes Jun 21, 2023
View reviewed changes
Copy link
Contributor

@clarkliming clarkliming left a comment

Choose a reason for hiding this comment

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

looks good to me!

@edelarua edelarua enabled auto-merge (squash) June 21, 2023 16:41
shajoezhu
shajoezhu approved these changes Jun 22, 2023
View reviewed changes
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.

lgtm! Thanks @edelarua

@edelarua edelarua merged commit 370a5e1 into main Jun 22, 2023
@edelarua edelarua deleted the 968_coxreg_allow_char@main branch June 22, 2023 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clarkliming clarkliming clarkliming approved these changes

@shajoezhu shajoezhu shajoezhu approved these changes

Assignees
No one assigned
Labels
sme
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Question]: should summarize_coxreg always require covariate variables be factors?
4 participants
@edelarua @shajoezhu @clarkliming @Melkiades