<br><br><div class="gmail_quote">On Wed, Nov 17, 2010 at 8:38 AM,  <span dir="ltr">&lt;<a href="mailto:josef.pktd@gmail.com">josef.pktd@gmail.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
On Tue, Nov 16, 2010 at 7:10 PM, Ralf Gommers<br>
<div class="im">&lt;<a href="mailto:ralf.gommers@googlemail.com">ralf.gommers@googlemail.com</a>&gt; wrote:<br>
&gt;<br>
&gt;<br>
</div><div><div></div><div class="h5">&gt; On Tue, Nov 16, 2010 at 11:45 PM, Bruce Southey &lt;<a href="mailto:bsouthey@gmail.com">bsouthey@gmail.com</a>&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; On 11/16/2010 07:04 AM, Ralf Gommers wrote:<br>
&gt;&gt;<br>
&gt;&gt; On Mon, Nov 15, 2010 at 12:40 AM, Bruce Southey &lt;<a href="mailto:bsouthey@gmail.com">bsouthey@gmail.com</a>&gt;<br>
&gt;&gt; wrote:<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; On Sat, Nov 13, 2010 at 8:50 PM,  &lt;<a href="mailto:josef.pktd@gmail.com">josef.pktd@gmail.com</a>&gt; wrote:<br>
&gt;&gt;&gt; &gt; <a href="http://projects.scipy.org/scipy/ticket/956" target="_blank">http://projects.scipy.org/scipy/ticket/956</a> and<br>
&gt;&gt;&gt; &gt; <a href="http://pypi.python.org/pypi/fisher/" target="_blank">http://pypi.python.org/pypi/fisher/</a> have Fisher&#39;s exact<br>
&gt;&gt;&gt; &gt; testimplementations.<br>
&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt; &gt; It would be nice to get a version in for 0.9. I spent a few<br>
&gt;&gt;&gt; &gt; unsuccessful days on it earlier this year. But since there are two new<br>
&gt;&gt;&gt; &gt; or corrected versions available, it looks like it just needs testing<br>
&gt;&gt;&gt; &gt; and a performance comparison.<br>
&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt; &gt; I won&#39;t have time for this, so if anyone volunteers for this, scipy<br>
&gt;&gt;&gt; &gt; 0.9 should be able to get Fisher&#39;s exact.<br>
&gt;&gt;&gt;<br>
&gt;&gt; <a href="https://github.com/rgommers/scipy/tree/fisher-exact" target="_blank">https://github.com/rgommers/scipy/tree/fisher-exact</a><br>
&gt;&gt; All tests pass. There&#39;s only one usable version (see below) so I didn&#39;t do<br>
&gt;&gt; performance comparison. I&#39;ll leave a note on #956 as well, saying we&#39;re<br>
&gt;&gt; discussing on-list.<br>
&gt;&gt;<br>
&gt;&gt;&gt; I briefly looked at the code at pypi link but I do not think it is<br>
&gt;&gt;&gt; good enough for scipy. Also, I do not like when people license code as<br>
&gt;&gt;&gt; &#39;BSD&#39; and there is a comment in cfisher.pyx  &#39;# some of this code is<br>
&gt;&gt;&gt; originally from the internet. (thanks)&#39;. Consequently we can not use<br>
&gt;&gt;&gt; that code.<br>
&gt;&gt;<br>
&gt;&gt; I agree, that&#39;s not usable. The plain Python algorithm is also fast enough<br>
&gt;&gt; that there&#39;s no need to bother with Cython.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; The code with ticket 956 still needs work especially in terms of the<br>
&gt;&gt;&gt; input types and probably the API (like having a function that allows<br>
&gt;&gt;&gt; the user to select either 1 or 2 tailed tests).<br>
&gt;&gt;<br>
&gt;&gt; Can you explain what you mean by work on input types? I used np.asarray<br>
&gt;&gt; and forced dtype to be int64. For the 1-tailed test, is it necessary? I note<br>
&gt;&gt; that pearsonr and spearmanr also only do 2-tailed.<br>
&gt;&gt;<br>
&gt;&gt; Cheers,<br>
&gt;&gt; Ralf<br>
&gt;&gt;<br>
&gt;&gt; I have no problem including this if we can agree on the API because<br>
&gt;&gt; everything else is internal that can be fixed by release date. So I would<br>
&gt;&gt; accept a place holder API that enable a user in the future to select which<br>
&gt;&gt; tail(s) is performed.<br>
&gt;<br>
&gt; It is always possible to add a keyword &quot;tail&quot; later that defaults to<br>
&gt; 2-tailed. As long as the behavior doesn&#39;t change this is perfectly fine, and<br>
&gt; better than having a placeholder.<br>
&gt;&gt;<br>
&gt;&gt; 1) It just can not use np.asarray() without checking the input first. This<br>
&gt;&gt; is particularly bad for masked arrays.<br>
&gt;&gt;<br>
&gt; Don&#39;t understand this. The input array is not returned, only used<br>
&gt; internally. And I can&#39;t think of doing anything reasonable with a 2x2 table<br>
&gt; with masked values. If that&#39;s possible at all, it should probably just go<br>
&gt; into mstats.<br>
&gt;<br>
&gt;&gt;<br>
&gt;&gt; 2) There are no dimension checking because, as I understand it, this can<br>
&gt;&gt; only handle a &#39;2 by 2&#39; table. I do not know enough for general &#39;r by c&#39;<br>
&gt;&gt; tables or the 1-d case either.<br>
&gt;&gt;<br>
&gt; Don&#39;t know how easy it would be to add larger tables. I can add dimension<br>
&gt; checking with an informative error message.<br>
<br>
</div></div>There is some discussion in the ticket about more than 2by2,<br>
additions would be nice (and there are some examples on the matlab<br>
fileexchange), but 2by2 is the most common case and has an unambiguous<br>
definition.<br>
<div class="im"><br>
<br>
&gt;<br>
&gt;&gt;<br>
&gt;&gt; 3) The odds-ratio should be removed because it is not part of the test. It<br>
&gt;&gt; is actually more general than this test.<br>
&gt;&gt;<br>
&gt; Don&#39;t feel strongly about this either way. It comes almost for free, and R<br>
&gt; seems to do the same.<br>
<br>
</div>same here, it&#39;s kind of traditional to return two things, but in this<br>
case the odds ratio is not the test statistic, but I don&#39;t see that it<br>
hurts either<br>
<div class="im"><br>
&gt;<br>
&gt;&gt; 4) Variable names such as min and max should not shadow Python functions.<br>
&gt;<br>
&gt; Yes, Josef noted this already, will change.<br>
&gt;&gt;<br>
&gt;&gt; 5) Is there a reference to the algorithm implemented? For example, SPSS<br>
&gt;&gt; provides a simple 2 by 2 algorithm:<br>
&gt;&gt;<br>
&gt;&gt; <a href="http://support.spss.com/ProductsExt/SPSS/Documentation/Statistics/algorithms/14.0/app05_sig_fisher_exact_test.pdf" target="_blank">http://support.spss.com/ProductsExt/SPSS/Documentation/Statistics/algorithms/14.0/app05_sig_fisher_exact_test.pdf</a><br>

&gt;<br>
&gt; Not supplied, will ask on the ticket and include it.<br>
<br>
</div>I thought, I saw it somewhere, but don&#39;t find the reference anymore,<br>
some kind of bisection algorithm, but having a reference would be<br>
good.<br>
Whatever the algorithm is, it&#39;s fast, even for larger values.<br>
<div class="im"><br>
&gt;&gt;<br>
&gt;&gt; 6) Why exactly does the dtype need to int64? That is, is there something<br>
&gt;&gt; wrong with hypergeom function? I just want to understand why the precision<br>
&gt;&gt; change is required because the input should enter with sufficient precision.<br>
&gt;&gt;<br>
&gt; This test:<br>
&gt; fisher_exact(np.array([[18000, 80000], [20000, 90000]]))<br>
&gt; becomes much slower and gives an overflow warning with int32. int32 is just<br>
&gt; not enough. This is just an implementation detail and does not in any way<br>
&gt; limit the accepted inputs, so I don&#39;t see a problem here.<br>
<br>
</div>for large numbers like this the chisquare test should give almost the<br>
same results, it looks pretty &quot;asymptotic&quot; to me. (the usual<br>
recommendation for the chisquare is more than 5 expected observations<br>
in each cell)<br>
I think the precision is required for some edge cases when<br>
probabilities get very small. The main failing case, I was fighting<br>
with for several days last winter, and didn&#39;t manage to fix had a zero<br>
at the first position. I didn&#39;t think about increasing the precision.<br>
<div class="im"><br>
&gt;<br>
&gt; Don&#39;t know what the behavior should be if a user passes in floats though?<br>
&gt; Just convert to int like now, or raise a warning?<br>
<br>
</div>I wouldn&#39;t do any type checking, and checking that floats are almost<br>
integers doesn&#39;t sound really necessary either, unless or until users<br>
complain. The standard usage should be pretty clear for contingency<br>
tables with count data.<br>
<br>
Josef<br>
<br>
</blockquote><div><br>Thanks for checking. <a href="https://github.com/rgommers/scipy/commit/b968ba17">https://github.com/rgommers/scipy/commit/b968ba17</a> should fix remaining things. Will wait for a few days to see if we get a reference to the algorithm. Then will commit.<br>
<br>Cheers,<br>Ralf<br><br></div></div><br>