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

Adds ard_regression_basic(). #62

Merged
merged 10 commits into from
Mar 1, 2024
Merged

Adds ard_regression_basic(). #62

merged 10 commits into from
Mar 1, 2024

Conversation

rparmm
Copy link
Contributor

@rparmm rparmm commented Feb 26, 2024

What changes are proposed in this pull request?

  • Adds ard_regression_basic()

Adds ard_regression_basic(), a wrapper function around the ard_regression S3 to provide basic statistics in ARD format for regression models.

closes #46


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 Feb 26, 2024

badge

Code Coverage Summary

Filename                    Stmts    Miss  Cover    Missing
------------------------  -------  ------  -------  ------------------------------------
R/ard_chisqtest.R              31       0  100.00%
R/ard_fishertest.R             33       0  100.00%
R/ard_kruskaltest.R            27       0  100.00%
R/ard_mcnemartest.R            41       0  100.00%
R/ard_moodtest.R               40       0  100.00%
R/ard_proportion_ci.R          40       5  87.50%   63-67
R/ard_regression_basic.R       14       1  92.86%   44
R/ard_regression.R             49       0  100.00%
R/ard_ttest.R                  78       0  100.00%
R/ard_wilcoxtest.R             83       0  100.00%
R/proportion_ci.R             188      28  85.11%   247, 250, 259-264, 272, 287, 387-410
TOTAL                         624      34  94.55%

Diff against main

Filename                    Stmts    Miss  Cover
------------------------  -------  ------  -------
R/ard_regression_basic.R      +14      +1  +92.86%
TOTAL                         +14      +1  -0.04%

Results for commit: 9d1944f

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Feb 26, 2024

Unit Tests Summary

 1 files  16 suites   2s ⏱️
16 tests 12 ✅  4 💤 0 ❌
35 runs  25 ✅ 10 💤 0 ❌

Results for commit 9d1944f.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Feb 26, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
ard_regression_basic 👶 $+0.12$ $+2$ $+1$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
ard_regression_basic 👶 $+0.12$ ard_regression_basic_works

Results for commit 6ff56cd

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 92.30769% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 99.63%. Comparing base (d74c53b) to head (ad3c09b).
Report is 1 commits behind head on main.

❗ Current head ad3c09b differs from pull request most recent head 54e4eaa. Consider uploading reports for the commit 54e4eaa to get more accurate results

Files Patch % Lines
R/ard_regression_basic.R 92.30% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
- Coverage   99.81%   99.63%   -0.18%     
==========================================
  Files          10       11       +1     
  Lines         534      547      +13     
==========================================
+ Hits          533      545      +12     
- Misses          1        2       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ddsjoberg
Copy link
Collaborator

thanks @rparmm ! taking a first look here. can you fill out the top portion of the review checklist and update so the spell check passes as well? let me know if you have any Qs!

@rparmm
Copy link
Contributor Author

rparmm commented Feb 27, 2024

thanks @rparmm ! taking a first look here. can you fill out the top portion of the review checklist and update so the spell check passes as well? let me know if you have any Qs!

I have been trying to fix that spell check. It seems like it is coming from the word ard. I cant find it anywhere in the package. Did you want me to add this to spell check to get it passed?

@ddsjoberg
Copy link
Collaborator

@rparmm I think it was coming from the help file title? i am not 100% sure, but I updated some text and it went away! i'll finish up the review later this week. Thanks!!

Copy link
Collaborator

@ddsjoberg ddsjoberg left a comment

Choose a reason for hiding this comment

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

thanks @rparmm !! i moved the function to its own script. looks great!

@ddsjoberg ddsjoberg merged commit b48d831 into main Mar 1, 2024
28 checks passed
@ddsjoberg ddsjoberg deleted the 46_ard_basic branch March 1, 2024 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do we need a basic regression model ARD?
3 participants