Hi Geoffrey,<br><br><div class="gmail_quote">On Tue, Dec 20, 2011 at 7:24 PM, Geoffrey Irving <span dir="ltr"><<a href="mailto:irving@naml.us">irving@naml.us</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello,<br>
<br>
As a followup to the prior thread on bugs in user defined types in<br>
numpy, I converted my rational number class from C++ to C and switched<br>
to 32 bits to remove the need for unportable 128 bit numbers.  It<br>
should be usable as a fairly thorough test case for user defined types<br>
now.  It does rather more than a minimal test case would need to do,<br>
but that isn't a problem unless you're concerned about code size.  Let<br>
me know if any further changes are needed before it's suitable for<br>
inclusion in numpy as a test case.  The repository is here:<br>
<br>
    <a href="https://github.com/girving/rational" target="_blank">https://github.com/girving/rational</a><br>
<br>
The tests run under either py.test or nose.<br>
<br>
For completeness, my branch fixing all but one of the bugs I found in<br>
numpy user defined types is here:<br>
<br>
    <a href="https://github.com/girving/numpy/tree/fixuserloops" target="_blank">https://github.com/girving/numpy/tree/fixuserloops</a><br>
<br>
The remaining bug is that numpy incorrectly releases the GIL during<br>
casts even though NPY_NEEDS_API is set.  The resulting crash goes away<br>
if the line defining ACQUIRE_GIL is uncommented.  With the necessary<br>
locks in place, all my tests pass with my branch of numpy.  I haven't<br>
tracked this one down and fixed it yet, but it shouldn't be hard to do<br>
so.<br>
<br></blockquote><div><br>A few preliminary comments on the C code (since I can't comment directly on github)<br><br>1) The C++ style comments aren't portable.<br><br>2) The trailing comments would (IMHO) look better on the line above.<br>
<br>3) The inline keyword isn't portable, use NPY_INLINE instead.<br><br>4) We've mostly used the<br><br style="font-family:courier new,monospace"><span style="font-family:courier new,monospace">    int</span><br style="font-family:courier new,monospace">
<span style="font-family:courier new,monospace">    foo(void)</span><br style="font-family:courier new,monospace"><span style="font-family:courier new,monospace">    {</span><br style="font-family:courier new,monospace"><span style="font-family:courier new,monospace">    }</span><br>
<br>style of function definition.<br><br>5) And for if statements<br><br><span style="font-family:courier new,monospace">    if (is_toohot) {</span><br style="font-family:courier new,monospace"><span style="font-family:courier new,monospace">        change_seats();</span><br style="font-family:courier new,monospace">
<span style="font-family:courier new,monospace">    }</span><br style="font-family:courier new,monospace"><span style="font-family:courier new,monospace">    else if (is_toocold) {</span><br style="font-family:courier new,monospace">
<span style="font-family:courier new,monospace">        change_seats();</span><br style="font-family:courier new,monospace"><span style="font-family:courier new,monospace">    }</span><br style="font-family:courier new,monospace">
<span style="font-family:courier new,monospace">    else {</span><br style="font-family:courier new,monospace"><span style="font-family:courier new,monospace">        eat_cereal();</span><br style="font-family:courier new,monospace">
<span style="font-family:courier new,monospace">    }</span><br><br>6) Because Python assert disappears in release code, the tests need to use<br>assert_(...) imported from numpy.testing<br><br>Chuck<br></div><br></div>