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

Refactor of as_result_df #954

Merged
merged 16 commits into from
Nov 20, 2024
Merged

Refactor of as_result_df #954

merged 16 commits into from
Nov 20, 2024

Conversation

Melkiades
Copy link
Contributor

@Melkiades Melkiades commented Nov 15, 2024

Fixes #948

It contains breaking changes @BFalquet @shajoezhu

NEWS (to add at the end)

  • Refactored as_result_df() to have a standard behavior, with all the relevant parameters, and a possibility to add personalized spec.
  • Removed result_df_specs(), because as_result_df() was a too shallow wrapper.
  • Merged behavior of as_result_df() parameters as_is and simplify parameters to remove structural information.
  • Initialized vignette about quality control outputs of as_result_df().
  • Refactored as_result_df() parameters as_strings and as_viewer into data_format = c("full_precision", "strings", "numeric") following the same outputs.
  • Initialized parameter make_ard output for single-line statistical outputs.

@shajoezhu
Copy link
Collaborator

shajoezhu commented Nov 15, 2024

let's test it in , raise PRs in the following packages, and with "empty commit" to trigger CICD tests @shajoezhu

@shajoezhu
Copy link
Collaborator

Refactored as_result_df() parameters as_strings and as_viewer into data_format = c("full_precision", "strings", "numeric") following the same outputs.

We don't have any dates summary right? as. min/max dates

@shajoezhu
Copy link
Collaborator

Initialized vignette about quality control outputs of as_result_df().
we can always come back and refine this

@shajoezhu
Copy link
Collaborator

hi @Melkiades , downstream all checks fine. I think we can merge this in for now. and start the other small tasks soon :)

@Melkiades Melkiades marked this pull request as ready for review November 18, 2024 08:43
Copy link
Contributor

github-actions bot commented Nov 18, 2024

Unit Tests Summary

    1 files     27 suites   1m 34s ⏱️
  211 tests   211 ✅ 0 💤 0 ❌
1 557 runs  1 557 ✅ 0 💤 0 ❌

Results for commit 6f029f8.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Nov 18, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
Tabulation framework 💔 $18.85$ $+1.48$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
Result Data Frames 💀 $1.03$ $-1.03$ as_result_df_as_is_is_producing_a_data.frame_that_is_compatible_with_df_to_tt
Result Data Frames 👶 $+1.11$ as_result_df_simplify_is_producing_a_data.frame_that_is_compatible_with_df_to_tt
Result Data Frames 💀 $1.17$ $-1.17$ as_result_df_works_with_visual_output_as_viewer_
Result Data Frames 👶 $+1.25$ as_result_df_works_with_visual_output_data_format_as_numeric_

Results for commit 0e028a9

♻️ This comment has been updated with latest results.

@Melkiades
Copy link
Contributor Author

Fixes also #953

Copy link
Contributor

github-actions bot commented Nov 18, 2024

badge

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
R/00tabletrees.R               780      63  91.92%   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, 1969-1970
R/as_html.R                    167      25  85.03%   5-10, 77, 149-154, 159-164, 179-183, 270
R/colby_constructors.R         594      26  95.62%   81, 134, 197-200, 267-270, 411, 427, 1177, 1265, 1426, 1465, 1476, 1484, 1487, 1511, 1532, 1678, 1901-1904
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            1119     140  87.49%   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
R/tt_afun_utils.R              411      32  92.21%   48, 155, 162, 171-184, 250, 258-259, 477, 485-488, 570-574, 594, 608-610
R/tt_as_df.R                   235      31  86.81%   60-62, 92-95, 150-153, 180-203, 217, 237, 349, 355, 387, 440
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                         8238     807  90.20%

Diff against main

Filename        Stmts    Miss  Cover
------------  -------  ------  -------
R/tt_as_df.R      +12     +14  -5.57%
TOTAL             +12     +14  -0.16%

Results for commit: 6f029f8

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@shajoezhu
Copy link
Collaborator

style and spell check @Melkiades

Copy link
Collaborator

@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 @Melkiades

@Melkiades Melkiades merged commit 989b946 into main Nov 20, 2024
30 checks passed
@Melkiades Melkiades deleted the 948_refactor_ard_functions@main branch November 20, 2024 08:38
@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor the way as_result_df handles custom functions
2 participants