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

Charles R Harris charlesr.harris at gmail.com
Sat Jun 13 14:22:10 EDT 2009


On Sat, Jun 13, 2009 at 12:00 PM, Charles R Harris <
charlesr.harris at gmail.com> wrote:

>
>
> On Sat, Jun 13, 2009 at 7:46 AM, David Cournapeau <
> david at ar.media.kyoto-u.ac.jp> wrote:
> >
> > Hi,
> >
> >    I have cleaned up a bit the code, and would like to suggest the
> > inclusion of a neighborhood iterator for numpy. Stéfan took a look at it
> > already, but it needs more eyeballs. It is a "subclass" of
> > PyArrayIterObject, and can be used to iterate over a neighborhood of a
> > point (handling boundaries with 0 padding for the time being).
> >
> > http://codereview.appspot.com/75055/show
> >
> > I have used it to replace the current for code correlateND in
> > scipy.signal, where it works quite well (I think it makes the code more
> > readable in that case).
>
> Some nitpicks:
>
> 1) The name neigh sounds like a horse. Maybe region,  neighborhood, or
> something similar would be better.
>
> 2) Is PyObject_Init NULL safe?
>
> ret = PyArray_malloc(sizeof(*ret));
> +    PyObject_Init((PyObject*)ret,&PyArrayNeighIter_Type);
> +    if (ret == NULL) {
> +        return NULL;
> +    }
>
> 3) Documentation is needed. In particular, I think it worth mentioning that
> the number of bounds is taken from the PyArrayIterObject, which isn't the
> most transparent thing.
>

More nitpicks:

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.

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?

3) Is it really worth the hassle to use inline functions? What does it buy
in terms of speed that justifies the complication?

Chuck

>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/numpy-discussion/attachments/20090613/5d282a91/attachment.html>


More information about the NumPy-Discussion mailing list