On Wed, Dec 21, 2011 at 1:37 PM, Geoffrey Irving <irv...@naml.us> wrote:
> On Wed, Dec 21, 2011 at 3:56 AM, Charles R Harris > <charlesr.har...@gmail.com> wrote: > > Hi Geoffrey, > > > > On Tue, Dec 20, 2011 at 7:24 PM, Geoffrey Irving <irv...@naml.us> wrote: > >> > >> Hello, > >> > >> As a followup to the prior thread on bugs in user defined types in > >> numpy, I converted my rational number class from C++ to C and switched > >> to 32 bits to remove the need for unportable 128 bit numbers. It > >> should be usable as a fairly thorough test case for user defined types > >> now. It does rather more than a minimal test case would need to do, > >> but that isn't a problem unless you're concerned about code size. Let > >> me know if any further changes are needed before it's suitable for > >> inclusion in numpy as a test case. The repository is here: > >> > >> https://github.com/girving/rational > >> > >> The tests run under either py.test or nose. > >> > >> For completeness, my branch fixing all but one of the bugs I found in > >> numpy user defined types is here: > >> > >> https://github.com/girving/numpy/tree/fixuserloops > >> > >> The remaining bug is that numpy incorrectly releases the GIL during > >> casts even though NPY_NEEDS_API is set. The resulting crash goes away > >> if the line defining ACQUIRE_GIL is uncommented. With the necessary > >> locks in place, all my tests pass with my branch of numpy. I haven't > >> tracked this one down and fixed it yet, but it shouldn't be hard to do > >> so. > >> > > > > A few preliminary comments on the C code (since I can't comment directly > on > > github) > > > > 1) The C++ style comments aren't portable. > > > > 2) The trailing comments would (IMHO) look better on the line above. > > > > 3) The inline keyword isn't portable, use NPY_INLINE instead. > > > > 4) We've mostly used the > > > > int > > foo(void) > > { > > } > > > > style of function definition. > > > > 5) And for if statements > > > > if (is_toohot) { > > change_seats(); > > } > > else if (is_toocold) { > > change_seats(); > > } > > else { > > eat_cereal(); > > } > > > > 6) Because Python assert disappears in release code, the tests need to > use > > assert_(...) imported from numpy.testing > > All fixed. > > Mark, I'm ready to merge pull-175, is there any reason not to? Chuck
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
