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

Zachary Pincus zachary.pincus at yale.edu
Sun Apr 19 09:55:29 EDT 2009


> I don't believe so.  I considered that, but there have to be some
> changes made to the C code to handle empty lists and better error
> checking, so figured I'd keep most of the changes in the same file.

I figured the wrapper could could just not even bother calling into C  
in the case of an empty list? But in any case, additional error  
checking is good.

> Each of the functions calling statistics() could wrap it with
> something that does the right thing with the return value given the
> type of index().  Let me know if you think that would be cleaner.

Not sure if it would be cleaner to keep it in python, but I figured  
there would be a lower potential for bugs. Since this patch is already  
written, it's probably easiest to use it, and I'll give it a close read.

Also the tickets Josef pointed out are related, and at least 868 needs  
fixing. Should those checks be in C or python? (I'm not sure what  
functional limitation is actually imposed by the bug in 412...)

Zach



> Ray
>
> On Sat, Apr 18, 2009 at 20:16, Zachary Pincus  
> <zachary.pincus at yale.edu> wrote:
>> I'm looking over the patch currently, but let me ask a quick question
>> first: is there a performance (or other) reason to not just do the
>> list-wrapping/unwrapping thing in Python, rather than in the C code?
>>
>> Zach
>>
>>
>>
>>
>> On Apr 18, 2009, at 7:28 PM, Thouis (Ray) Jones wrote:
>>
>>> My last patch did not do the right thing for ndimage.extrema(),  
>>> which
>>> needs to return 4 values with the same semantics.  I've attached an
>>> addendum.
>>>
>>> Ray Jones
>>>
>>>
>>> On Sat, Apr 18, 2009 at 16:47, Thouis (Ray) Jones <thouis at broad.mit.edu
>>>> wrote:
>>>> Here's a first attempt at a patch.  It fixes the corner case of a
>>>> single index to match the index type (scalar for scalar, list for
>>>> single element sequence), also the case of index=[] to return []  
>>>> for
>>>> any measurement.  Finally, it improves type checking of the index
>>>> type, which was only being checked for sum() and not any of the  
>>>> other
>>>> measurements (?).
>>>>
>>>> Zach, would you mind reviewing?  If it looks good, I'll add some
>>>> tests
>>>> for the new functionality and submit a patch via trac.
>>>>
>>>> Ray
>>>>
>>>> On Fri, Apr 17, 2009 at 22:06, Zachary Pincus <zachary.pincus at yale.edu
>>>>> wrote:
>>>>>> Current behavior:
>>>>>>>>> from scipy import ndimage
>>>>>>>>> ndimage.maximum([[1,2], [3,4]], [[1,0],[0,2]], index=[1,2])
>>>>>> [1.0, 4.0]
>>>>>>>>> ndimage.maximum([[1,2], [3,4]], [[1,0],[0,2]], index=[1])
>>>>>> 1.0
>>>>>>>>> ndimage.maximum([[1,2], [3,4]], [[1,0],[0,2]], index=1)
>>>>>> 1.0
>>>>>>
>>>>>> I think the second result should be [1.0].  We're using the  
>>>>>> ndimage
>>>>>> code for image processing, and have to wrap the measurement
>>>>>> functions
>>>>>> to make the code general for the case where there is only one
>>>>>> object
>>>>>> in the image.
>>>>>
>>>>> I've run into this too, and had the same thought. It's such a
>>>>> corner-
>>>>> case that it doesn't seem like gratuitous API-breakage to make  
>>>>> this
>>>>> fix...
>>>>>
>>>>> Zach
>>>>> _______________________________________________
>>>>> Scipy-dev mailing list
>>>>> Scipy-dev at scipy.org
>>>>> http://mail.scipy.org/mailman/listinfo/scipy-dev
>>>>>
>>>>
>>> <0002-special-case-for-
>>> extrema.patch>_______________________________________________
>>> Scipy-dev mailing list
>>> Scipy-dev at scipy.org
>>> http://mail.scipy.org/mailman/listinfo/scipy-dev
>>
>> _______________________________________________
>> Scipy-dev mailing list
>> Scipy-dev at scipy.org
>> http://mail.scipy.org/mailman/listinfo/scipy-dev
>>
> _______________________________________________
> Scipy-dev mailing list
> Scipy-dev at scipy.org
> http://mail.scipy.org/mailman/listinfo/scipy-dev




More information about the SciPy-Dev mailing list