BUG: Failing type return on masked array on LHS

[cpelley]

Self contained unittest which I have added and shown to fail (demonstrating what I believe to be a bug). Essentially, the mask is thrown away.

>>> arr = np.ma.array([1, 2, 3], mask=[False, True, False])
>>> barr = biggus.NumpyArrayAdapter(arr)
>>> print np.array([[ 1.]]) * barr
[[  1.00000000e+00   9.99999000e+05   3.00000000e+00]]
>>> print np.array([[ 1.]]) * arr
[[1.0 -- 3.0]]

I suspect it's because its the __mul__ from the masked numpy array that is being called and for whatever reason, it doesn't think the biggus array is masked.

[pelson]

This can be fixed by adding the __array_priority__ to the NumpyArrayAdapter:

diff --git a/biggus/__init__.py b/biggus/__init__.py
index bb7c181..2a8c4c9 100644
--- a/biggus/__init__.py
+++ b/biggus/__init__.py
@@ -737,6 +737,10 @@ class ArrayContainer(Array):
         except AttributeError:
             return np.array(self.array)

+    @property
+    def __array_priority__(self):
+        return self.array.__array_priority__
+
     def masked_array(self):
         try:
             return self.array.masked_array()
@@ -1306,6 +1310,10 @@ class _ArrayAdapter(Array):
     def shape(self):
         return _sliced_shape(self.concrete.shape, self._keys)

+    @property
+    def __array_priority__(self):
+        return self.concrete.__array_priority__
+
     def _cleanup_new_key(self, key, size, axis):
         """
         Return a key of type int, slice, or tuple that is guaranteed

@cpelley - interested in taking this forwards?

[cpelley]

@cpelley - interested in taking this forwards?

Thanks for your interest @pelson. I'm glad that I'm not just being silly (I was sure that something must have run into this before now, but hey..).

Happy to make the change myself. Thanks for pointing out the solution. Is it OK to be calling this private attribute within the numpy object? (not that we have any choice). Is there a discussion to raise to have numpy expose this via the public interface following this ticket?

Cheers

[pelson]

Is there a discussion to raise to have numpy expose this via the public interface following this ticket?

Double underscore doesn't really mean private, it is generally a indication of "interface". In truth, even if it did mean private, it is not something that can be readily changed in numpy - we are stuck with that with older versions anyway, so any change would have to have additional logic above an beyond what we would currently need to do. All in all, I don't believe raising a numpy ticket is a worthwhile pursuit in all honesty.

If you're happy to carry this forwards, I'd be happy to review any tests added.

[cpelley]

If you're happy to carry this forwards, I'd be happy to review any tests added.

Sure, thanks @pelson

[cpelley]

The solution doesn't work for numpy v1.7 for whatever reason and we are supporting v1.7 for biggus for some reason. Since iris requires numpy >= 1.9, I propose we have travis run tests for biggus with numpy v1.9 of numpy rather than v1.7.

What do you think?

[marqh]

@cpelley please rebase based on #193

[cpelley]

@cpelley please rebase based on #193

193 hasn't been merged into master so I'll simply merge it into this branch I suppose and close #193. Any reason why #193 wasn't simply merged independently?

[DPeterK]

I'll simply merge it into this branch I suppose

Argh that's horrible, don't do that! Let's just be patient and get #193 merged, and then this one can go in. Can you undo the recent rebase please @cpelley?

[cpelley]

OK done

[cpelley]

Hello, just checking that this doesn't get missed for the v1.11 iris milestone.

Cheers

[cpelley]

@dkillick @marqh here is some numpy documentation on the __array_priority__ attribute if that helps. As always, let me know if I can do anything to help.

Cheers

[pelson]

👍 from me. Small, simple change to fix a very specific issue. I think we have had plenty of time to review the change, and no major issues have cropped up, so I'll go ahead and merge.

[pelson]

Actually, would you mind catching the case where the contained arrays don't have an __array_priority. We don't document this as a prerequisite for containing arrays, so we shouldn't raise an exception if they don't have one.

[cpelley]

Hey @pelson, thanks for looking at this. I have now added a test case which uses a ConstantArray and pass that to a NumpyArrayAdapter as the ConstantArray class does not define an __array_priority__ attribute. Hmm can't we define this property on the Array parent class?

[pelson]

Turns out numpy must be catching this AttributeError anyways. Thanks for adding the test @cpelley.

[cpelley]

Thanks @pelson and thanks for pointing me in the right direction on fixing this. Glad to see this nasty bug disappear :)

[pelson]

Thank you @cpelley.

We need to cut a release of biggus at some point. I have a few substantial changes that can probably wait for the next (major) release 😉