On Tue, Oct 19, 2010 at 2:24 AM, Pauli Virtanen <pav@iki.fi> wrote:
Tue, 19 Oct 2010 01:09:54 -0600, Charles R Harris wrote: [clip]
Just a quick look. I wasn't able to add comments to the code, maybe a pull request would allow that or maybe you need to enable something.
I think you can only add comments on commits, not in Github's compare view.
There are links to the commits at the top of the compare view, where the commenting appears to work. <snip>
But perhaps we should just recommend people filing pull requests right away?
Maybe describing one canonical way to do things (for newcomers, at least), would make things clear? Currently http://docs.scipy.org/doc/numpy/dev/gitwash/development_workflow.html#basic-... <http://docs.scipy.org/doc/numpy/dev/gitwash/development_workflow.html#basic-workflow>says "ask for a code review or make a pull request," so I chose the first option.
I'm not sure how broadcasting is supposed to work for
structured arrays so I will leave that to someone else. ISTR that the SUN compiler is persnickety about the initialization of structures, only accepts constants or some such. I'll try to track that down or maybe someone here who is familiar with that compiler can comment.
I suspect that this comparison code should refuse to compare arrays with shape(a) != shape(b), even if `a` and `b` are broadcastable to one another. The issue is that the broadcasting semantics work on the array level, but here the boolean sub-array is implicitly reduced to a single boolean.
I believe this would already be caught by the dtype comparison earlier in array_richcompare, here: http://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/arrayobj... You can also initialize
dimensions = shape(self) + (-1,)
and let PyArray_Newshape do the size calculation for you.
I guess it's best to not initialize structures directly, if there is some suspicion that obscure compilers don't like it.
I made both of these changes, visible here: http://github.com/m-paradox/numpy/compare/master...fix_structured_compare It looks like to make a pull request, I need to merge my branch back into master. Is that correct, or should I do a pull request from the branch? Either way, adding a note here about that would be helpful: http://docs.scipy.org/doc/numpy/dev/gitwash/development_workflow.html#filing... -Mark