[Numpy-discussion] possible bug: __array_wrap__ is not called during arithmetic operations in some cases

[Darren Dale]

On Sun, Mar 8, 2009 at 6:04 PM, Charles R Harris
<charlesr.harris at gmail.com>wrote:

>
>
> On Sun, Mar 8, 2009 at 3:27 PM, Darren Dale <dsdale24 at gmail.com> wrote:
>
>> On Sun, Mar 8, 2009 at 5:02 PM, Darren Dale <dsdale24 at gmail.com> wrote:
>>
>>> On Sun, Mar 8, 2009 at 4:54 PM, Charles R Harris <
>>> charlesr.harris at gmail.com> wrote:
>>>
>>>>
>>>>
>>>> On Sun, Mar 8, 2009 at 2:48 PM, Charles R Harris <
>>>> charlesr.harris at gmail.com> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Sun, Mar 8, 2009 at 1:04 PM, Darren Dale <dsdale24 at gmail.com>wrote:
>>>>>
>>>>>> On Sat, Mar 7, 2009 at 1:23 PM, Darren Dale <dsdale24 at gmail.com>wrote:
>>>>>>
>>>>>>> On Sun, Feb 22, 2009 at 7:01 PM, Darren Dale <dsdale24 at gmail.com>wrote:
>>>>>>>
>>>>>>>> On Sun, Feb 22, 2009 at 6:35 PM, Darren Dale <dsdale24 at gmail.com>wrote:
>>>>>>>>
>>>>>>>>> On Sun, Feb 22, 2009 at 6:28 PM, Pierre GM <pgmdevlist at gmail.com>wrote:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Feb 22, 2009, at 6:21 PM, Eric Firing wrote:
>>>>>>>>>>
>>>>>>>>>> > Darren Dale wrote:
>>>>>>>>>> >> Does anyone know why __array_wrap__ is not called for
>>>>>>>>>> subclasses
>>>>>>>>>> >> during
>>>>>>>>>> >> arithmetic operations where an iterable like a list or tuple
>>>>>>>>>> >> appears to
>>>>>>>>>> >> the right of the subclass? When I do "mine*[1,2,3]", array_wrap
>>>>>>>>>> is
>>>>>>>>>> >> not
>>>>>>>>>> >> called and I get an ndarray instead of a MyArray.
>>>>>>>>>> "[1,2,3]*mine" is
>>>>>>>>>> >> fine, as is "mine*array([1,2,3])". I see the same issue with
>>>>>>>>>> >> division,
>>>>>>>>>> >
>>>>>>>>>> > The masked array subclass does not show this behavior:
>>>>>>>>>>
>>>>>>>>>> Because MaskedArray.__mul__ and others are redefined.
>>>>>>>>>>
>>>>>>>>>> Darren, you can fix your problem by redefining MyArray.__mul__ as:
>>>>>>>>>>
>>>>>>>>>>     def __mul__(self, other):
>>>>>>>>>>         return np.ndarray.__mul__(self, np.asanyarray(other))
>>>>>>>>>>
>>>>>>>>>> forcing the second term to be a ndarray (or a subclass of). You
>>>>>>>>>> can do
>>>>>>>>>> the same thing for the other functions (__add__, __radd__, ...)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks for the suggestion. I know this can be done, but ufuncs like
>>>>>>>>> np.multiply(mine,[1,2,3]) will still not work. Plus, if I reimplement these
>>>>>>>>> methods, I take some small performance hit. I've been putting a lot of work
>>>>>>>>> in lately to get quantities to work with numpy's stock ufuncs.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I should point out:
>>>>>>>>
>>>>>>>> import numpy as np
>>>>>>>>
>>>>>>>> a=np.array([1,2,3,4])
>>>>>>>> b=np.ma.masked_where(a>2,a)
>>>>>>>> np.multiply([1,2,3,4],b) # yields a masked array
>>>>>>>> np.multiply(b,[1,2,3,4]) # yields an ndarray
>>>>>>>>
>>>>>>>>
>>>>>>> I'm not familiar with the numpy codebase, could anyone help me figure
>>>>>>> out where I should look to try to fix this bug? I've got a nice set of
>>>>>>> generators that work with nosetools to test all combinations of numerical
>>>>>>> dtypes, including combinations of scalars, arrays, and iterables of each
>>>>>>> type. In my quantities package, just testing multiplication yields 1031
>>>>>>> failures, all of which appear to be caused by this bug (#1026 on trak) or
>>>>>>> bug #826.
>>>>>>
>>>>>>
>>>>>>
>>>>>> I finally managed to track done the source of this problem.
>>>>>> _find_array_wrap steps through the inputs, asking each of them for their
>>>>>> __array_wrap__ and binding it to wrap. If more than one input defines
>>>>>> __array_wrap__, you enter a block that selects one based on array priority,
>>>>>> and binds it back to wrap. The problem was when the first input defines
>>>>>> array_wrap but the second one does not. In that case, _find_array_wrap never
>>>>>> bothered to rebind the desired wraps[0] to wrap, so wrap remains Null or
>>>>>> None, and wrap is what is returned to the calling function.
>>>>>>
>>>>>> I've tested numpy with this patch applied, and didn't see any
>>>>>> regressions. Would someone please consider committing it?
>>>>>>
>>>>>> Thanks,
>>>>>> Darren
>>>>>>
>>>>>> $ svn diff numpy/core/src/umath_ufunc_object.inc
>>>>>> Index: numpy/core/src/umath_ufunc_object.inc
>>>>>> ===================================================================
>>>>>> --- numpy/core/src/umath_ufunc_object.inc       (revision 6569)
>>>>>> +++ numpy/core/src/umath_ufunc_object.inc       (working copy)
>>>>>> @@ -3173,8 +3173,10 @@
>>>>>>              PyErr_Clear();
>>>>>>          }
>>>>>>      }
>>>>>> +    if (np >= 1) {
>>>>>> +        wrap = wraps[0];
>>>>>> +    }
>>>>>>      if (np >= 2) {
>>>>>> -        wrap = wraps[0];
>>>>>>          maxpriority = PyArray_GetPriority(with_wrap[0],
>>>>>>                                          PyArray_SUBTYPE_PRIORITY);
>>>>>>          for (i = 1; i < np; ++i) {
>>>>>>
>>>>>
>>>>> Applied in r6573. Thanks.
>>>>>
>>>>
>>>> Oh, and can you provide a test for this fix?
>>>>
>>>
>>> Yes, I'll send a patch for a test as soon as its ready. 6573 closes two
>>> tickets, 1026 and 1022. Did you see the patch I sent for issue #826? It is
>>> also posted at the bug report.
>>
>>
>>
>> Index: numpy/core/tests/test_umath.py
>> ===================================================================
>> --- numpy/core/tests/test_umath.py      (revision 6573)
>> +++ numpy/core/tests/test_umath.py      (working copy)
>> @@ -240,6 +240,19 @@
>>          assert_equal(args[1], a)
>>          self.failUnlessEqual(i, 0)
>>
>> +    def test_wrap_with_iterable(self):
>> +        # test fix for bug #1026:
>> +        class with_wrap(np.ndarray):
>> +            __array_priority = 10
>> +            def __new__(cls):
>> +                return np.asarray(1).view(cls).copy()
>> +            def __array_wrap__(self, arr, context):
>> +                return arr.view(type(self))
>> +        a = with_wrap()
>> +        x = ncu.multiply(a, (1, 2, 3))
>> +        self.failUnless(isinstance(x, with_wrap))
>> +        assert_array_equal(x, np.array((1, 2, 3)))
>> +
>>      def test_old_wrap(self):
>>          class with_wrap(object):
>>              def __array__(self):
>>
>
> Thanks. This was applied in r6575.
>

Chuck, I'm sorry, there was a typo in that test. It should have said
__array_priority__, not __array_priority. It didnt influence the test
result, which failed without the patch and passed with it, but I think it
should still be fixed.

Darren
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/numpy-discussion/attachments/20090308/4da979fb/attachment.html>