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

[Charles R Harris]

Hi David,

On Wed, Jun 17, 2009 at 10:35 PM, David Cournapeau<cournape at gmail.com> wrote:
> On Mon, Jun 15, 2009 at 1:45 AM, Charles R
> Harris<charlesr.harris at gmail.com> wrote:
>>
>> 1) The documentation of PyObject_Init doesn't say whether it is NULL
>> safe, so I think there needs to be a check here before the call:
>
> I checked the code of PyObject_init: I think it is safe to call it
> with NULL, since NULL is checked for. I was lazy to change this, as I
> should then change other objects which do this as well in numpy for
> consistency.
>
>> 2) Do the bounds need to be ordered? If so, that should be mentioned
>> and checked.
>
> I added this to the documentation (in the ref guide of numpy)
>
>>
>> 3) In the documentation x is used but the function prototype uses iter.
>
> Fixed.
>
>>
>> 4) I don't think the reference is borrowed since it is incremented if
>> the ctor succeeds. I think the point here is that the user doesn't
>> need to worry about it.
>
> I have always been confused by borrowed/stolen vocabulary, to be
> honest. I mentioned that nothing is changed if the ctor fails, and
> that the neighborhood holds a new reference.
>
>> 5) There should be spaces around the "-" here:
>> for (i = iter->nd-1; i >= 0; --i)
>>  Likewise, the convention in python seems to be a space between the
>> "for" and "("
>
> done.
>
>> 6) If the functions use neighborhood (I do think that looks better),
>> then the file names should also.
>
> In numpy, that's integrated in iterators.c + one header. I will commit
> the code to numpy, once I have checked it works as expected in scipy.
>

OK, I'm out of nits ;)

> That raises the question: can we make scipy 0.8.0 depends on numpy
> 1.4.0, or should I maintain a copy of the iterator in scipy 0.8.x so
> that it can be compiled with numpy 1.3.0 ?
>

I don't know. I think it is cleaner to depend on 1.4.0, but there
might be other considerations such as release schedules, both for
numpy/scipy and perhaps some of the major linux distros
(Fedora/Ubuntu). You are probably a better judge of that than I, but
we should discuss it along with setting some goals for the next
release.

Chuck