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

Adding stat_names and support for row/col splits and ContentRow #958

Merged
merged 19 commits into from
Nov 29, 2024

Conversation

Melkiades
Copy link
Contributor

@Melkiades Melkiades commented Nov 21, 2024

#950
#947
#949

NEWS

  • Completed make_ard = TRUE support for as_result_df().
  • Added stat_names to rcell() to be used by as_result_df(make_ard = TRUE).

@Melkiades Melkiades marked this pull request as ready for review November 21, 2024 14:53
Copy link
Contributor

github-actions bot commented Nov 21, 2024

Unit Tests Summary

    1 files     28 suites   1m 36s ⏱️
  217 tests   217 ✅ 0 💤 0 ❌
1 577 runs  1 577 ✅ 0 💤 0 ❌

Results for commit 948d68d.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Nov 21, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
Result Data Frames 💔 $4.45$ $+2.74$ $+18$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
Result Data Frames 👶 $+0.16$ make_ard_produces_realistic_ARD_output_with_as_result_df
Result Data Frames 👶 $+0.05$ make_ard_works_if_there_are_no_stat_names
Result Data Frames 👶 $+0.31$ make_ard_works_with_multiple_column_levels
Result Data Frames 👶 $+0.40$ make_ard_works_with_multiple_row_levels
Result Data Frames 👶 $+1.72$ make_ard_works_with_summarize_row_groups

Results for commit 17224b0

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Nov 21, 2024

badge

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/00tabletrees.R               781      63  91.93%   20, 94, 97, 428, 519-520, 523, 681, 785, 877-878, 980, 983, 985-986, 1004-1007, 1027, 1142-1145, 1243-1248, 1404, 1504-1507, 1573-1576, 1612-1615, 1621-1626, 1677, 1684, 1778, 1886, 1899, 1902-1905, 1908-1911, 1938, 1970-1971
R/as_html.R                    167      25  85.03%   5-10, 77, 149-154, 159-164, 179-183, 270
R/colby_constructors.R         597      26  95.64%   81, 134, 197-200, 267-270, 411, 427, 1181, 1269, 1430, 1469, 1480, 1488, 1491, 1516, 1537, 1683, 1906-1909
R/compare_rtables.R             83      17  79.52%   93-96, 99-102, 115-118, 137, 156-157, 188, 193
R/custom_split_funs.R          265      40  84.91%   127, 132, 138-143, 156, 173-177, 353-358, 375-380, 456, 502, 518-521, 537, 599, 609-610, 612, 624, 668, 693
R/default_split_funs.R         286      22  92.31%   271, 334-337, 348-349, 351, 353, 550-554, 618-621, 684-687
R/format_rcell.R                12       0  100.00%
R/indent.R                      13       2  84.62%   40-41
R/index_footnotes.R             66       0  100.00%
R/make_split_fun.R             138      31  77.54%   22-26, 36-39, 52-55, 58-61, 115, 119, 267, 270-273, 278-281, 295, 366, 375, 377, 379, 430
R/make_subset_expr.R           137      15  89.05%   35, 47-61, 135-142, 178, 267, 271, 280
R/summary.R                    144      38  73.61%   35, 80, 178-220, 269, 315-331, 366, 397
R/tree_accessors.R            1125     141  87.47%   110, 139-140, 264, 284, 310, 333, 363, 381, 400-404, 426, 448-451, 566, 593-594, 880-886, 1030, 1049, 1075, 1127, 1184-1185, 1222, 1257, 1295-1300, 1359, 1433-1437, 1455-1464, 1542, 1662-1665, 1690, 1712-1713, 1723, 1774, 1795-1800, 1821-1826, 1837, 1911, 1952, 2051, 2158, 2171, 2185, 2201, 2210, 2220-2224, 2274-2279, 2482, 2492-2495, 2505, 2530-2533, 2540, 2542-2545, 2667, 2701-2702, 2759, 3064, 3425, 3541, 3575-3600, 3691-3699, 3852, 3926-3932, 4144-4145, 4152, 4155-4158, 4162, 4212, 4273, 4298-4322, 4351
R/tt_afun_utils.R              417      33  92.09%   57, 178, 185, 194-208, 276, 284-285, 503, 511-514, 596-600, 620, 634-636
R/tt_as_df.R                   292      14  95.21%   80-83, 150-153, 278, 332, 444, 450, 482, 535
R/tt_compare_tables.R           70       4  94.29%   51, 174, 246, 250
R/tt_compatibility.R           570      70  87.72%   19, 142-143, 186, 191, 319-320, 324-327, 333, 337, 521, 575-578, 615-617, 655, 688, 708, 728-731, 741-744, 789, 806-810, 816-819, 893, 920-923, 932, 994, 1002, 1013-1016, 1127, 1134, 1162-1176, 1207-1208
R/tt_dotabulation.R           1161      95  91.82%   54, 246, 251, 253, 301, 325, 329-332, 364-367, 390, 423-426, 454-457, 552, 690-694, 743, 747, 775-778, 788, 808-812, 819-822, 1082, 1086, 1117, 1220-1223, 1433-1441, 1705-1714, 1796-1799, 1810, 1815, 1820-1821, 1823, 1834, 1839, 1862, 1948-1967
R/tt_export.R                   13       1  92.31%   45
R/tt_from_df.R                  15       0  100.00%
R/tt_paginate.R                513      37  92.79%   74, 122-131, 441, 576-579, 600-604, 749-752, 803-810, 887, 890, 908, 915, 918
R/tt_pos_and_access.R          571      42  92.64%   76, 78-80, 105, 166, 212-216, 258, 507, 509, 517, 523, 537, 547-550, 730, 733, 741-745, 750-753, 780, 833-834, 845, 1007-1008, 1076-1092, 1362, 1437
R/tt_showmethods.R             162      21  87.04%   56, 91-113, 223, 249, 258, 263, 266-270, 359-360
R/tt_sort.R                    101       5  95.05%   245-248, 256
R/tt_toString.R                433      24  94.46%   123, 345, 367, 380, 390, 396, 399, 405-415, 508, 609, 811-836
R/utils.R                       34       7  79.41%   56, 169-174
R/validate_table_struct.R       84      10  88.10%   80-84, 93-94, 140, 149-150
R/Viewer.R                      61       9  85.25%   46, 50, 60-64, 84, 118
TOTAL                         8311     792  90.47%

Diff against main

Filename                  Stmts    Miss  Cover
----------------------  -------  ------  -------
R/00tabletrees.R             +1       0  +0.01%
R/colby_constructors.R       +2       0  +0.01%
R/tree_accessors.R           +6      +1  -0.02%
R/tt_afun_utils.R            +6      +1  -0.13%
R/tt_as_df.R                +57     -17  +8.40%
TOTAL                       +72     -15  +0.27%

Results for commit: 4cd4edb

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@Melkiades
Copy link
Contributor Author

@edelarua @ayogasekaram @BFalquet this is ready for review ;) Thanks!!

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,

Thanks for working on this - it looks great!! I have a few questions/observations, but I'm not sure how closely we're trying to match the output from {cards} so some of it may not be relevant.

I also made a few updates to the ARD vignette but it's just spelling/grammar stuff (no code changes). Let me know if you have any questions about that.

R/tt_afun_utils.R Outdated Show resolved Hide resolved
tests/testthat/test-result_data_frame.R Outdated Show resolved Hide resolved
tests/testthat/test-result_data_frame.R Outdated Show resolved Hide resolved
tests/testthat/test-result_data_frame.R Outdated Show resolved Hide resolved
tests/testthat/test-result_data_frame.R Outdated Show resolved Hide resolved
tests/testthat/test-result_data_frame.R Outdated Show resolved Hide resolved
R/tt_as_df.R Outdated Show resolved Hide resolved
@edelarua edelarua self-assigned this Nov 28, 2024
Melkiades and others added 3 commits November 28, 2024 11:45
Co-authored-by: Emily de la Rua <[email protected]>
Signed-off-by: Davide Garolini <[email protected]>
Co-authored-by: Emily de la Rua <[email protected]>
Signed-off-by: Davide Garolini <[email protected]>
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.

I think this looks good now! Thanks for the updates @Melkiades. Don't forget the styler :)

@Melkiades Melkiades enabled auto-merge (squash) November 29, 2024 10:11
@Melkiades Melkiades merged commit f87f1ef into main Nov 29, 2024
28 checks passed
@Melkiades Melkiades deleted the adding_stats_names_to_ards@main branch November 29, 2024 10:21
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants