[Numpy-discussion] patch for structured array comparison bug

Mark Wiebe mwwiebe@gmail....
Tue Oct 19 14:00:54 CDT 2010


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-workflow

<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/arrayobject.c#L932

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-a-pull-request


-Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.scipy.org/pipermail/numpy-discussion/attachments/20101019/32366092/attachment.html 


More information about the NumPy-Discussion mailing list