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

Add method for Fischer's exact test #395

Merged
merged 11 commits into from
Oct 22, 2023
Merged

Add method for Fischer's exact test #395

merged 11 commits into from
Oct 22, 2023

Conversation

rempsyc
Copy link
Member

@rempsyc rempsyc commented Sep 30, 2023

Add full support for Fischer's exact test: closes #351


# load packages
library(report)
packageVersion("report")
#> [1] '0.5.7.11'

# Create data and fischer test object
TeaTasting <-
  matrix(c(3, 1, 1, 3),
         nrow = 2,
         dimnames = list(Guess = c("Milk", "Tea"),
                         Truth = c("Milk", "Tea")))
x <- fisher.test(TeaTasting, alternative = "greater")

report(x)
#> Effect sizes were labelled following Funder's (2019) recommendations.
#> 
#> The Fisher's Exact Test for Count Data testing the association between the
#> variables of the TeaTasting dataset suggests that the effect is statistically
#> not significant, and large (odds ratio = 6.41, 95% CI [0.31, Inf], p = 0.243;
#> Adjusted Cramer's v = 0.35, 95% CI [0.00, 1.00])

Created on 2023-09-30 with reprex v2.0.2

@codecov
Copy link

codecov bot commented Sep 30, 2023

Codecov Report

Merging #395 (75ef67a) into main (da67740) will decrease coverage by 0.03%.
The diff coverage is 75.64%.

❗ Current head 75ef67a differs from pull request most recent head ecaabb4. Consider uploading reports for the commit ecaabb4 to get more accurate results

@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
- Coverage   73.81%   73.78%   -0.03%     
==========================================
  Files          48       49       +1     
  Lines        3425     3494      +69     
==========================================
+ Hits         2528     2578      +50     
- Misses        897      916      +19     
Files Coverage Δ
R/report.htest.R 92.02% <100.00%> (+0.35%) ⬆️
R/report_htest_fisher.R 71.21% <71.21%> (ø)

... and 1 file with indirect coverage changes

Copy link
Member

@IndrajeetPatil IndrajeetPatil left a comment

Choose a reason for hiding this comment

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

Can you please also clean the newly found lints?

@rempsyc
Copy link
Member Author

rempsyc commented Oct 22, 2023

Can you please also clean the newly found lints?

@IndrajeetPatil I ran the tests locally and found no problems with the lints with the changed files after downloading today's version of lintr from github. Then I triggered the tests on GHA and again it seems both lint workflows pass. So is it OK?

@rempsyc
Copy link
Member Author

rempsyc commented Oct 22, 2023

Ok there are snapshots failing now (is that what you meant?). On R-devel, it seems that Message is now printed as <simpleMessage>, e.g.,

- Message <simpleMessage>
+ Message

And so my understanding is that it is not currently possible to satisfy both R-devel and R-latest with snapshots. In those cases, should I suppress messages?

@IndrajeetPatil
Copy link
Member

IndrajeetPatil commented Oct 22, 2023 via email

@rempsyc
Copy link
Member Author

rempsyc commented Oct 22, 2023

Oh nice, amazing, thank you. Then updating testthat to latest now I am able to update the snapshots correctly locally :)

@rempsyc
Copy link
Member Author

rempsyc commented Oct 22, 2023

Ok so I fixed snapshots, but check-random-test-order and test-coverage are still failing.

For check-random-test-order, this is strange because the file contains a single test_that() call... I think I remember seeing a similar discussion at some point in our other repos...

For test-coverage, the issue seems to be with rstan::stan_model, so not sure what is going on and cannot replicate locally (where all tests pass). I even ran the workflow covr::codecov(quiet = FALSE) and that too works, so not sure what to do here.

That's after installing all latest easystats packages with easystats::install_latest() and also updating all my other packages to their latest version.

@rempsyc
Copy link
Member Author

rempsyc commented Oct 22, 2023

Ok, all tests are passing now, I think I can merge after approval :)

Copy link
Member

@IndrajeetPatil IndrajeetPatil left a comment

Choose a reason for hiding this comment

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

Thanks!

@IndrajeetPatil IndrajeetPatil merged commit bca974c into main Oct 22, 2023
26 checks passed
@IndrajeetPatil IndrajeetPatil deleted the fisher branch October 22, 2023 20:08
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.

Support Fisher exact test reporting
2 participants