On Tue, Oct 19, 2010 at 1:00 PM, Mark Wiebe <[email protected]> wrote:
> On Tue, Oct 19, 2010 at 2:24 AM, Pauli Virtanen <[email protected]> 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. > > We're still working things out after the move, so you get to be sort of a guinea pig ;) I think a pull request is the way to go. > > 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 > > > You can request a pull of the branch. Just give it a shot, you can always delete the request. You might want to rebase off the master branch first, however. Chuck > - >
_______________________________________________ NumPy-Discussion mailing list [email protected] http://mail.scipy.org/mailman/listinfo/numpy-discussion
