-
Notifications
You must be signed in to change notification settings - Fork 88
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 Constructor to FisherExactTest #156
base: master
Are you sure you want to change the base?
Conversation
Add constructor to accept a Matrix as argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Can you add a test for the new constructor?
Also, while you're at it, why not accept two vectors x
and y
and compute the contingency table using counts
just like in the Chi-squared test?
src/fisher.jl
Outdated
@@ -41,10 +45,11 @@ The contingency table is structured as: | |||
|*Y1*| a | b | | |||
|*Y2*| c | d | | |||
|
|||
!!! note | |||
!!! Note: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is syntax, not text, so please keep it as-is.
src/fisher.jl
Outdated
The `show` function output contains the conditional maximum likelihood estimate of the | ||
odds ratio rather than the sample odds ratio; it maximizes the likelihood given by | ||
Fisher's non-central hypergeometric distribution. | ||
The entries must be non-negative integers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Say this in the sentence about contingency table rather than in this note, which is about something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made all the suggested changes.
Add the suggested changes (constructor for vectors and update documentation).
|
||
Perform Fisher's exact test of the null hypothesis that the success probabilities ``a/c`` | ||
and ``b/d`` are equal, that is the odds ratio ``(a/c) / (b/d)`` is one, against the | ||
alternative hypothesis that they are not equal. | ||
|
||
If `x` is a matrix with at least two rows and columns, it is taken as a two-dimensional | ||
contingency table. Otherwise, `x` and `y` must be vectors of the same length. The contingency | ||
table is calculated using `counts` function from the `StatsBase` package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
table is calculated using `counts` function from the `StatsBase` package. | |
table is calculated using the `counts` function from the `StatsBase` package. |
FisherExactTest(x[1,1], x[1,2], x[2,1], x[2,2]) | ||
end | ||
|
||
function FisherExactTest(x::AbstractVector{T}, y::AbstractVector{T}) where T<:Integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function FisherExactTest(x::AbstractVector{T}, y::AbstractVector{T}) where T<:Integer | |
function FisherExactTest(x::AbstractVector{<:Integer}, y::AbstractVector{<:Integer}) |
@@ -73,6 +79,15 @@ struct FisherExactTest <: HypothesisTest | |||
end | |||
end | |||
|
|||
function FisherExactTest(x::AbstractMatrix{T}) where T<:Integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function FisherExactTest(x::AbstractMatrix{T}) where T<:Integer | |
function FisherExactTest(x::AbstractMatrix{<:Integer}) |
|
||
d = counts(x,y) | ||
|
||
FisherExactTest(d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check that results are the same for both calls, and that they are equal to what you get when passing a
, b
, c
and d
manually.
Bump. |
I recently reported an issue #154, since fisher's test works with contingency tables it would be expected to accept a matrix as argument, however it is not implemented. This commit resolves this problem.