[SciPy-dev] Should ndimage.measurements.* should return lists if index is a list?

[josef.pktd at gmail.com]

On Mon, Apr 20, 2009 at 2:26 AM, David Cournapeau <
david at ar.media.kyoto-u.ac.jp> wrote:

> Hi Josef,
>
> josef.pktd at gmail.com wrote:
> > There are not many complaints/tickets about the code itself, no wrong
> > "images", in contrast to other subpackages of scipy.
>
> If there are crashes that nobody can fix, that's a problem in the long
> term. If we have code which is slower than the current version, but in
> cython and well written, then people can improve it for speed later. If
> the code is bad enough so that nobody fixes it, then it will never be
> improved. If ndimage has a good testsuite, then it is possible to
> replace C code with cython (it can be done gradually). In my experience,
> maintainable and reliable C code for python extensions is hard, and not
> that many people are capable of doing it, myself included. There is also
> the issue of portability to Py3k.
>
> I am afraid that a lot of code in scipy is in that stage that it is
> useful but not that maintainable - I like the "paying the technical
> debt" metaphor (http://en.wikipedia.org/wiki/Technical_debt), and I
> think we are exactly in that period where we have accumulated a lot of
> debt, ndimage being such an example.
>

I looked a bit more on the segfault with correlate, ticket:295

To raise an exception for the case that currently segfaults, we would only
need to change one inequality

filters.py line 489 (def _correlate_or_convolve)
-  if (lenw // 2 + origin < 0) or (lenw // 2 + origin > lenw):
+ if (lenw // 2 + origin < 0) or (lenw // 2 + origin >= lenw):

however, the case that causes the problem is actually a valid case,
correlate1d has no problems with it

python -c "from scipy import ndimage; print
ndimage.correlate1d([0,1,2,3,4,5],[1,1,2,0],origin=2)"
[7 3 1 2 5 9]

changing the inequality would now raise an exception in this case (instead
of crash):

python -c "from scipy import ndimage; print
ndimage.correlate([0,1,2,3,4,5],[1,1,2,0],origin=2)"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File
"C:\Josef\eclipsegworkspace\scipybranch2\dist\Programs\Python25\Lib\site-
packages\scipy\ndimage\filters.py", line 524, in correlate
    origin, False)
  File
"C:\Josef\eclipsegworkspace\scipybranch2\dist\Programs\Python25\Lib\site-
packages\scipy\ndimage\filters.py", line 497, in _correlate_or_convolve
    raise ValueError, 'invalid origin'
ValueError: invalid origin

(note: my line numbers are different because of debug print statements,
tested for 1d and 2d case)

 Since the segfault occurs at a valid case, I tried to check ni_filters.c,
but after staring at it for a while, I don't see where (presumably) the
memory allocation goes wrong, and given that it requires compilation it's
not so easy to just play with it and debug it like python code. I'm just
glad I never had to learn c/c++.

So, I completely agree, if we have to look for bugs in the c code or if
porting to py3 doesn't work automatically, this code is a big technical
debt.

But in the mean time, we could convert these segfaults to exceptions.

Josef
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scipy-dev/attachments/20090420/bf707e7c/attachment.html>