github-actions[bot]
commented
2 months ago ✅ All contributors have signed the CLA
Posted by the CLA Assistant Lite bot.
Closed iaugusty closed 5 days ago
github-actions[bot]
commented
2 months ago ✅ All contributors have signed the CLA
Posted by the CLA Assistant Lite bot.
github-actions[bot]
commented
2 months ago 1 files 84 suites 1m 13s ⏱️ 870 tests 859 ✅ 11 💤 0 ❌ 1 867 runs 1 171 ✅ 696 💤 0 ❌
Results for commit cc1f58ee.
:recycle: This comment has been updated with latest results.
github-actions[bot]
commented
2 months ago | Test Suite | $Status$ | Time on main |
$±Time$ | $±Tests$ | $±Skipped$ | $±Failures$ | $±Errors$ |
|---|---|---|---|---|---|---|---|
| analyze_vars_in_cols | 💔 | $2.38$ | $+3.28$ | $+17$ | $-7$ | $0$ | $0$ |
| count_occurrences | 💔 | $0.74$ | $+1.59$ | $+10$ | $-8$ | $0$ | $0$ |
| count_occurrences_by_grade | 💔 | $1.74$ | $+1.09$ | $+16$ | $-17$ | $0$ | $0$ |
| summarize_coxreg | 💔 | $3.80$ | $+2.48$ | $+13$ | $-13$ | $0$ | $0$ |
| summarize_num_patients | 💔 | $1.06$ | $+1.33$ | $+18$ | $-16$ | $0$ | $0$ |
Results for commit c94458d2920039c9a846bea145bc47fb896ee57c
♻️ This comment has been updated with latest results.
github-actions[bot]
commented
2 months ago 
Filename Stmts Miss Cover Missing
--------------------------------------- ------- ------ ------- ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/abnormal_by_baseline.R 65 0 100.00%
R/abnormal_by_marked.R 55 5 90.91% 93-97
R/abnormal_by_worst_grade_worsen.R 116 3 97.41% 263-265
R/abnormal_by_worst_grade.R 60 0 100.00%
R/abnormal.R 43 0 100.00%
R/analyze_variables.R 175 2 98.86% 497, 637
R/analyze_vars_in_cols.R 176 13 92.61% 178, 221, 235-236, 244-252
R/bland_altman.R 92 1 98.91% 46
R/combination_function.R 9 0 100.00%
R/compare_variables.R 84 2 97.62% 257, 316
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 59 1 98.31% 74
R/count_missed_doses.R 36 0 100.00%
R/count_occurrences_by_grade.R 157 2 98.73% 177, 271
R/count_occurrences.R 116 1 99.14% 120
R/count_patients_events_in_cols.R 67 1 98.51% 60
R/count_patients_with_event.R 62 1 98.39% 123
R/count_patients_with_flags.R 95 1 98.95% 134
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% 65
R/estimate_proportion.R 205 11 94.63% 83-90, 94, 99, 320, 486
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, 632, 707, 716, 722, 741, 796-816, 819, 830, 849, 904, 907, 1042-1047
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 46 0 100.00%
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 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 0 100.00%
R/incidence_rate.R 86 7 91.86% 67-72, 152
R/logistic_regression.R 102 0 100.00%
R/missing_data.R 21 3 85.71% 32, 66, 76
R/odds_ratio.R 117 0 100.00%
R/prop_diff_test.R 91 0 100.00%
R/prop_diff.R 265 15 94.34% 70-73, 105, 290-297, 440, 605
R/prune_occurrences.R 57 0 100.00%
R/response_biomarkers_subgroups.R 69 6 91.30% 196-201
R/response_subgroups.R 213 8 96.24% 100-105, 260-261
R/riskdiff.R 65 5 92.31% 102-105, 114
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 106 2 98.11% 183, 188
R/summarize_change.R 74 1 98.65% 184
R/summarize_colvars.R 10 0 100.00%
R/summarize_coxreg.R 172 0 100.00%
R/summarize_glm_count.R 209 3 98.56% 193-194, 490
R/summarize_num_patients.R 93 4 95.70% 117-119, 266
R/summarize_patients_exposure_in_cols.R 96 1 98.96% 56
R/survival_biomarkers_subgroups.R 78 6 92.31% 117-122
R/survival_coxph_pairwise.R 84 12 85.71% 51-52, 64-73
R/survival_duration_subgroups.R 211 6 97.16% 124-129
R/survival_time.R 111 0 100.00%
R/survival_timepoint.R 124 10 91.94% 131-140
R/utils_checkmate.R 68 0 100.00%
R/utils_default_stats_formats_labels.R 157 0 100.00%
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 124 9 92.74% 39, 46, 403-404, 526-530
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 10805 473 95.62%Filename Stmts Miss Cover
-------------------------------------- ------- ------ --------
R/analyze_variables.R +9 0 +0.06%
R/survival_coxph_pairwise.R +5 +1 -0.36%
R/survival_time.R +32 0 +100.00%
R/survival_timepoint.R +11 +3 -1.87%
R/utils_default_stats_formats_labels.R +12 0 +100.00%
TOTAL +69 +4 -0.01%Results for commit: cc1f58ee3f5439d057770aeb0d8ed28ca6f8e77c
Minimum allowed coverage is 80%
:recycle: This comment has been updated with latest results
iaugusty
commented
2 months ago I have read the CLA Document and I hereby sign the CLA
Melkiades
commented
2 months ago Pull Request
Fixes #1301 @Melkiades I'm not sure about the impact of adding new stats to an analyze function on the remainder of the code/packages. Could you review and share your thoughts? This is the first function for which we'd like to add extra stats, there will be more functions for which we'd like to combine the stat and it's confidence interval into 1 line. Once we know the approach we can follow, more of these types of updates might follow.
For the moment, it looks good. I will check downstream if there are breaking changes due to using all possible values by default but in principle that should be changed. @shajoezhu @edelarua, what do you think about freely adding stats like this?
@iaugusty, we are already trying to introduce a bit more flexibility in custom stats' addition, so for me, it is practically good to go. Lets hear the others ;)
edelarua
commented
2 months ago @edelarua, what do you think about freely adding stats like this?
Sounds fine to me, I would just try to stick with naming conventions used throughout the package as much as possible for any added statistics :)
Melkiades
commented
2 months ago Lgtm anyway! @iaugusty could you just fix the checks? Thanks
shajoezhu
commented
2 months ago block this PR by https://github.com/insightsengineering/tern/pull/1311
Melkiades
commented
2 months ago @iaugusty could you update the snapshots please? Thanks!!
iaugusty
commented
1 month ago @iaugusty could you update the snapshots please? Thanks!!
When I try to run the tests, it seems I have to update the svg files for the graphs, for font changes? Should I have some special settings to ensure I match the fonts that are being used at your end?
I added the long version also to s_summary.numeric, as median_long was added to utils_default_stats_formats_labels.R, and this is being utilized by s_summary.numeric as well. This triggered me to add this stat in s_summary.numeric, and as we want the same long version for mean there as well, I added it at the same time. I hope this is fine within this same branch.
Melkiades
commented
1 month ago @iaugusty could you update the snapshots please? Thanks!!
When I try to run the tests, it seems I have to update the svg files for the graphs, for font changes? Should I have some special settings to ensure I match the fonts that are being used at your end?
I added the long version also to s_summary.numeric, as median_long was added to utils_default_stats_formats_labels.R, and this is being utilized by s_summary.numeric as well. This triggered me to add this stat in s_summary.numeric, and as we want the same long version for mean there as well, I added it at the same time. I hope this is fine within this same branch.
You can turn off the plot diff with the following (in test/setup.R):
# expect_snapshot_ggplot - set custom plot dimensions
expect_snapshot_ggplot <- function(title, fig, width = NA, height = NA) {
testthat::skip()
testthat::skip_on_ci()
testthat::skip_if_not_installed("svglite")Looking now at mean_long etc I think it would be better to have mean_ci etc as it is exactly that. Thanks Ilse!!
mean_ci is already in, which is just the CI of the mean, not including the mean, while mean_long is a 3-d stat: mean + CI
iaugusty
commented
1 month ago @Melkiades : not sure if you saw my reply on your comment Looking now at mean_long etc I think it would be better to have mean_ci etc as it is exactly that. Thanks Ilse!!
mean_ci is already in, which is just the CI of the mean, not including the mean, while mean_long is a 3-d stat: mean + CI
Melkiades
commented
1 month ago @Melkiades : not sure if you saw my reply on your comment Looking now at mean_long etc I think it would be better to have mean_ci etc as it is exactly that. Thanks Ilse!!
mean_ci is already in, which is just the CI of the mean, not including the mean, while mean_long is a 3-d stat: mean + CI
I see! I would call it mean_and_ci or mean_ci_3d then ;)
shajoezhu
commented
1 month ago hi @iaugusty , I was wondering if you could update and resolve this conflict please? the PR is mostly fine to be merged now. Thanks!
shajoezhu
commented
1 month ago hi @iaugusty , I was wondering could you do the following please.
Melkiades
commented
1 month ago hi @iaugusty , I was wondering could you do the following please.
- resolve the conflict
- in https://github.com/insightsengineering/scda.test, on branch 1301-feature-request-add-confidence-intervals-for-quantiles-in-surv_time, so the test will kick off (1301 feature request add confidence intervals for quantiles in surv time scda.test#157 I have done this for you. lets wait and see)
tests pass!! @iaugusty could you rerun the documentation to solve conflict? thanks :) then we merge
Pull Request
Fixes #1301 @melkiades I'm not sure about the impact of adding new stats to an analyze function on the remainder of the code/packages. Could you review and share your thoughts? This is the first function for which we'd like to add extra stats, there will be more functions for which we'd like to combine the stat and it's confidence interval into 1 line. Once we know the approach we can follow, more of these types of updates might follow.