[Numpy-discussion] Ready for review: PyArrayNeighIterObject, an iterator to iterate over a neighborhood in arbitrary arrays

[David Cournapeau]

On Sun, Jun 14, 2009 at 4:59 PM, David Cournapeau<cournape at gmail.com> wrote:
> On Sun, Jun 14, 2009 at 3:51 AM, Charles R
> Harris<charlesr.harris at gmail.com> wrote:
>>
>>
>> On Sat, Jun 13, 2009 at 12:35 PM, David Cournapeau <cournape at gmail.com>
>> wrote:
>>>
>>> On Sun, Jun 14, 2009 at 3:22 AM, Charles R
>>> Harris<charlesr.harris at gmail.com> wrote:
>>>
>>> > 1) Since reference counting is such a pain, you should document that the
>>> > constructor returns a new reference and that the PyArrayIterObject does
>>> > not
>>> > need to have its reference count incremented before the call and that
>>> > the
>>> > reference count is unchanged on failure.
>>>
>>> OK.
>>>
>>> > 2) Why are _update_coord_iter(c) and _inc_set_ptr(c) macros? Why are
>>> > they
>>> > defined inside functions? If left as macros, they should be in CAPS, but
>>> > why
>>> > not just write them out?
>>>
>>> They are macro because they are reused in the 2d specialized functions
>>> (I will add 3d too)
>>
>> IIRC, inline doesn't recurse, so there is some advantage to having these as
>> macros. But I really dislike seeing macros defined inside of functions,
>> especially when they aren't exclusive to that function.
>
> Well, they are kind of exclusive to this function - and the special 2d
> case; they are not supposed to be used by themselves (which is why
> they are undefined right away). But I changed this anyway to ALL CAPS
> and defined outside.
>
>> IOW, use macros judiciously.
>
> It may not be obvious, because it looks really simple, but the code is
> heavily optimized. I spent several hours to find an implementation
> which works as it does now. The macro is used for a reason :)

Forgot a link to the updated version:

http://github.com/cournape/scipy3/blob/1a4a6b4619a5d4f954168a02ed485db1a3b6b8e8/scipy/signal/neighiter.h

cheers,

David