Fix `a_summary()` all-`NA` factor bug #1432

iaugusty · 2025-08-12T15:06:10Z

Pull Request

Fixes #1431

github-actions · 2025-08-12T15:18:09Z

Unit Tests Summary

1 files 85 suites 1m 21s ⏱️
888 tests 879 ✅ 9 💤 0 ❌
1 907 runs 1 207 ✅ 700 💤 0 ❌

Results for commit 891f9fa.

♻️ This comment has been updated with latest results.

github-actions · 2025-08-12T15:18:15Z

Unit Test Performance Difference

Additional test case details

Test Suite	$Status$	Time on `main`	$±Time$	Test Case
analyze_variables	👶		$+0.05$	analyze_vars_works_with_all_NA_character_input
analyze_variables	👶		$+0.05$	analyze_vars_works_with_all_NA_factor_input

Results for commit d5c69fa

♻️ This comment has been updated with latest results.

github-actions · 2025-08-12T15:20:46Z

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                        288       5  98.26%   586-589, 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               169       8  95.27%   178, 386, 388, 465, 467, 469, 473-474
R/count_occurrences.R                        137      10  92.70%   119, 262-264, 330-332, 334, 338-339
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                      240       7  97.08%   88, 99, 255, 257-258, 389, 553
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%   222, 396-403, 442-452, 561, 569
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%   250, 252
R/prop_diff.R                                318      17  94.65%   71-74, 106, 299, 301, 387-394, 537, 702
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                       252      16  93.65%   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                         150       2  98.67%   327-328
R/summarize_change.R                          72       3  95.83%   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                   121      10  91.74%   122-124, 244, 248, 252-253, 337-338, 340
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              250      15  94.00%   124-129, 268-273, 286, 288-289
R/survival_time.R                            120       1  99.17%   251
R/survival_timepoint.R                       153       2  98.69%   320, 322
R/utils_checkmate.R                           68       0  100.00%
R/utils_default_stats_formats_labels.R       196       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, 414-415, 537-541
R/utils_split_funs.R                          52       2  96.15%   82, 94
R/utils.R                                    143       7  95.10%   131, 134, 137, 141, 150-151, 345
TOTAL                                      11969     580  95.15%

Diff against main

Filename                 Stmts    Miss  Cover
---------------------  -------  ------  -------
R/analyze_variables.R       +1       0  +0.01%
TOTAL                       +1       0  +0.00%

Results for commit: 891f9fa

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

R/analyze_variables.R

edelarua · 2025-08-26T22:51:37Z

Hi @iaugusty,

Thanks for the PR! I have updated your solution to simplify it a bit. All tests are passing but feel free to double check that this still works for you.

edelarua

This should be good to go after a final review by @iaugusty

Melkiades · 2025-08-28T08:37:54Z

This should be good to go after a final review by @iaugusty

Thanks Emily for helping us finding a neat solution! :)

shajoezhu · 2025-08-28T08:44:11Z

Thanks for the fix! @edelarua . @iaugusty can you please add tests to scda.test, we will merge this PR after the snapshot test is introduced. Thanks!

iaugusty · 2025-08-28T12:15:13Z

@edelarua indeed more elegant solution, thanks
@shajoezhu I'll take a look on how to add a test for this in scda.test

iaugusty · 2025-08-28T12:34:59Z

@shajoezhu could you provide some guidance on how to include a test for this scenario in scda.test, shall I eg include it as a special variant situation in eg test-table_dmt01.R, or would you prefer a separate program for a simple situation with this scenario? Any naming convention I need to follow, or any other guidance documentation you have would be helpfull.

shajoezhu · 2025-08-29T08:29:32Z

hi @iaugusty

Please do the following

at https://github.com/insightsengineering/scda.test, create a branch and raise PR, consider using example from, but the test case should demonstrate what you would like to do for this example, https://github.com/insightsengineering/scda.test/blob/main/tests/testthat/test-table_dmt01.R
for your test, we can give it a name say test-table_jnj_tb1.R we can discuss with better naming in the future.
change https://github.com/insightsengineering/scda.test/blob/main/DESCRIPTION#L50 to insightsengineering/tern@1431-bug-s_summaryfactor
change https://github.com/insightsengineering/scda.test/blob/main/DESCRIPTION#L28 to tern (>= 0.9.9.9002)

first draft

f7d2966

iaugusty linked an issue Aug 12, 2025 that may be closed by this pull request

[Bug]: s_summary.factor when only missing values/level is present in data and option na_rm = TRUE #1431

Open

3 tasks

[skip style] [skip vbump] Restyle files

fa72b14

Merge branch 'main' into 1431-bug-s_summaryfactor

30674a2

iaugusty requested a review from Melkiades August 12, 2025 15:58

Melkiades reviewed Aug 13, 2025

View reviewed changes

R/analyze_variables.R Outdated Show resolved Hide resolved

iaugusty added 2 commits August 14, 2025 08:19

update after suggestion

927b04f

fix linter

33effa8

iaugusty changed the title ~~first attempt to resolve bugfix~~ address bugfix Aug 14, 2025

Simplify solution

5fd210c

edelarua added 2 commits August 26, 2025 19:06

Clean up, comment

e0572b7

Remove last bit of extra code

4bbe87d

edelarua changed the title ~~address bugfix~~ Fix a_summary() all-NA factor bug Aug 26, 2025

edelarua added the sme label Aug 26, 2025

edelarua approved these changes Aug 26, 2025

View reviewed changes

edelarua added 2 commits August 27, 2025 15:54

Merge branch 'main' into 1431-bug-s_summaryfactor

af20a6b

Merge main - update NEWS

891f9fa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix `a_summary()` all-`NA` factor bug #1432

Fix `a_summary()` all-`NA` factor bug #1432

Uh oh!

iaugusty commented Aug 12, 2025

Uh oh!

github-actions bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 12, 2025 •

edited

Loading

Uh oh!

Uh oh!

edelarua commented Aug 26, 2025

Uh oh!

edelarua left a comment

Uh oh!

Melkiades commented Aug 28, 2025

Uh oh!

shajoezhu commented Aug 28, 2025

Uh oh!

iaugusty commented Aug 28, 2025

Uh oh!

iaugusty commented Aug 28, 2025

Uh oh!

shajoezhu commented Aug 29, 2025

Uh oh!

Uh oh!

Uh oh!

Fix a_summary() all-NA factor bug #1432

Are you sure you want to change the base?

Fix a_summary() all-NA factor bug #1432

Uh oh!

Conversation

iaugusty commented Aug 12, 2025

Pull Request

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests Summary

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Performance Difference

Uh oh!

github-actions bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Diff against main

Uh oh!

Uh oh!

edelarua commented Aug 26, 2025

Uh oh!

edelarua left a comment

Choose a reason for hiding this comment

Uh oh!

Melkiades commented Aug 28, 2025

Uh oh!

shajoezhu commented Aug 28, 2025

Uh oh!

iaugusty commented Aug 28, 2025

Uh oh!

iaugusty commented Aug 28, 2025

Uh oh!

shajoezhu commented Aug 29, 2025

Uh oh!

Uh oh!

Fix `a_summary()` all-`NA` factor bug #1432

Fix `a_summary()` all-`NA` factor bug #1432

github-actions bot commented Aug 12, 2025 •

edited

Loading

github-actions bot commented Aug 12, 2025 •

edited

Loading

github-actions bot commented Aug 12, 2025 •

edited

Loading