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 as_cards_fn() function #361

Merged
merged 7 commits into from
Nov 26, 2024
Merged

Adding as_cards_fn() function #361

merged 7 commits into from
Nov 26, 2024

Conversation

ddsjoberg
Copy link
Collaborator

@ddsjoberg ddsjoberg commented Nov 26, 2024

What changes are proposed in this pull request?

  • Added functions as_cards_fn(), is_cards_fn(), and get_cards_fn_stat_names(). These functions assist is creating functions with attributes enumerating the expected results.
  • Updated ard_continuous() and ard_complex() to return full ARDs when functions passed are created with as_cards_fn(): instead of a single row output, we get a long ARD with rows for each of the expected statistic names. (How to specify the expected returned statistics for cases when there are errors? #316)

Reference GitHub issue associated with pull request. e.g., 'closes #'
closes #316

NOTE: We had spoken about including an argument for specifying the formatting function as well, which is not included. It turns out the fmt_fn are assigned a very different part of the function, and it would have required so strange gymnastics to utilize this information. Also, both ard_continuous() and ard_complex() have a fmt_fn argument where the formatting functions can be specified.

After this update is live, we can also begin updating a bunch of {cardx} functions, so when errors occur we continue to get the structure of ARD we expect without errors as well. Most ARD functions wrap either ard_continuous() and ard_complex() internally FYI


Pre-review Checklist (if item does not apply, mark is as complete)

  • All GitHub Action workflows pass with a ✅
  • PR branch has pulled the most recent updates from master branch: usethis::pr_merge_main()
  • If a bug was fixed, a unit test was added.
  • Code coverage is suitable for any new functions/features (generally, 100% coverage for new code): devtools::test_coverage()
  • Request a reviewer

Reviewer Checklist (if item does not apply, mark is as complete)

  • If a bug was fixed, a unit test was added.
  • Run pkgdown::build_site(). Check the R console for errors, and review the rendered website.
  • Code coverage is suitable for any new functions/features: devtools::test_coverage()

When the branch is ready to be merged:

  • Update NEWS.md with the changes from this pull request under the heading "# cards (development version)". If there is an issue associated with the pull request, reference it in parentheses at the end update (see NEWS.md for examples).
  • All GitHub Action workflows pass with a ✅
  • Approve Pull Request
  • Merge the PR. Please use "Squash and merge" or "Rebase and merge".

Copy link
Contributor

github-actions bot commented Nov 26, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
as_cards_fn 👶 $+0.05$ $+3$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
add_calculated_row 💚 $1.21$ $-1.05$ add_calculated_row_x_
ard_categorical 💚 $9.13$ $-1.05$ ard_categorical_univariate
ard_complex 👶 $+0.01$ ard_complex_with_as_cards_fn_inputs
ard_continuous 👶 $+0.01$ ard_continuous_with_as_cards_fn_inputs
ard_continuous 💔 $4.20$ $+2.68$ ard_continuous_works
ard_hierarchical 💚 $9.75$ $-1.72$ ard_hierarchical_works_without_by_variables
ard_stack 💔 $1.40$ $+5.04$ ard_stack_works
ard_stack_hierarchical 💔 $0.58$ $+5.10$ ard_stack_hierarchical_variables_
ard_strata 💚 $1.93$ $-1.24$ ard_strata_works
as_cards_fn 👶 $+0.05$ as_cards_fn_works

Results for commit 2947ba4

♻️ This comment has been updated with latest results.

Copy link
Contributor

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
as_cards_fn 👶 $+0.04$ $+3$ $0$ $0$ $0$
selectors 💔 $0.09$ $+1.25$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
add_calculated_row 💔 $0.05$ $+1.24$ add_calculated_row_x_
ard_categorical 💔 $7.76$ $+1.18$ ard_categorical_univariate
ard_complex 👶 $+0.00$ ard_complex_with_as_cards_fn_inputs
ard_continuous 👶 $+0.00$ ard_continuous_with_as_cards_fn_inputs
ard_continuous 💚 $3.58$ $-2.92$ ard_continuous_works
ard_hierarchical 💔 $7.67$ $+1.83$ ard_hierarchical_works_without_by_variables
ard_stack 💔 $4.87$ $+1.57$ ard_stack_works
ard_stack_hierarchical 💔 $0.07$ $+5.97$ ard_stack_hierarchical_variables_
ard_strata 💔 $0.50$ $+2.36$ ard_strata_works
as_cards_fn 👶 $+0.04$ as_cards_fn_works
selectors 💔 $0.09$ $+1.25$ selectors_work

Results for commit d069f10

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Nov 26, 2024

Unit Tests Summary

  1 files  178 suites   48s ⏱️
175 tests  93 ✅  82 💤 0 ❌
327 runs  226 ✅ 101 💤 0 ❌

Results for commit 13aa0c4.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Nov 26, 2024

badge

Code Coverage Summary

Filename                       Stmts    Miss  Cover    Missing
---------------------------  -------  ------  -------  -------------------------------
R/add_calculated_row.R            46       0  100.00%
R/apply_fmt_fn.R                 106       0  100.00%
R/ard_attributes.R                45       1  97.78%   57
R/ard_categorical.R              350      10  97.14%   238, 395-399, 406-407, 583, 617
R/ard_complex.R                   36       1  97.22%   83
R/ard_continuous.R               186       3  98.39%   207-208, 307
R/ard_dichotomous.R               80       1  98.75%   62
R/ard_hierarchical.R              72       0  100.00%
R/ard_missing.R                   58       1  98.28%   50
R/ard_pairwise.R                  46       0  100.00%
R/ard_stack_hierarchical.R       232       1  99.57%   455
R/ard_stack.R                     90       1  98.89%   117
R/ard_strata.R                    33       0  100.00%
R/ard_total_n.R                   10       0  100.00%
R/as_card_fn.R                     8       0  100.00%
R/as_card.R                        5       0  100.00%
R/as_nested_list.R                41       0  100.00%
R/bind_ard.R                      47      11  76.60%   74-85
R/check_ard_structure.R           39       6  84.62%   31, 55-59
R/default_stat_labels.R           18       0  100.00%
R/eval_capture_conditions.R       30       0  100.00%
R/get_ard_statistics.R            16       0  100.00%
R/mock.R                         137       2  98.54%   116, 244
R/nest_for_ard.R                  71       1  98.59%   60
R/print_ard_conditions.R          78       0  100.00%
R/print.R                         80       0  100.00%
R/process_selectors.R            126       1  99.21%   337
R/replace_null_statistic.R        11       0  100.00%
R/round5.R                         1       0  100.00%
R/selectors.R                     19       0  100.00%
R/shift_ard_columns.R            102       1  99.02%   185
R/shuffle_ard.R                  129       0  100.00%
R/summary_functions.R             25       1  96.00%   59
R/tidy_ard_order.R                26       0  100.00%
R/tidy_as_ard.R                   39       0  100.00%
R/update_ard.R                    53       0  100.00%
R/utils.R                         24       0  100.00%
TOTAL                           2515      42  98.33%

Diff against main

Filename              Stmts    Miss  Cover
------------------  -------  ------  --------
R/ard_continuous.R       +8       0  +0.07%
R/as_card_fn.R           +8       0  +100.00%
TOTAL                   +16       0  +0.01%

Results for commit: 13aa0c4

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Collaborator

@bzkrouse bzkrouse left a comment

Choose a reason for hiding this comment

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

@ddsjoberg Love how simple this is!!

@ddsjoberg
Copy link
Collaborator Author

@ddsjoberg Love how simple this is!!

YES! I was excited to see how simple it was to implement! 👯

@jtalboys while writing the article about creating ARD functions for statistical methods we haven't added to cardx, you'll want to include the use of this functionality. You'll be the first to use it in practice, so please note anything you feel could be a smoother user experience!

@ddsjoberg ddsjoberg merged commit c8fcf18 into main Nov 26, 2024
35 checks passed
@ddsjoberg ddsjoberg deleted the 316-as_card_fn branch November 26, 2024 20:34
@github-actions github-actions bot locked and limited conversation to collaborators Nov 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to specify the expected returned statistics for cases when there are errors?
2 participants