-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG Ensure NotImplemented is passed on in MaskedArray ufunc's #3900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Needs tests. |
Needs tests.
|
not having contributed to numpy before, I'm not familiar with its Prof. M. H. van Kerkwijk |
All you need to do is install nose. After that the easiest way to proceed is |
@charris - thanks, that probably saved me hours! Now added test cases. |
@@ -1662,6 +1662,26 @@ def test_ndarray_mask(self): | |||
assert_equal(test.mask, control.mask) | |||
self.assertTrue(not isinstance(test.mask, MaskedArray)) | |||
|
|||
def test_treatment_of_NotImplemented(self): | |||
"Check we return NotImplemented if ufunc cannot deal with other" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For nose related reasons, don't use docstrings in tests, just do # ...
Looking good, just a few issues. |
This will need a backport to both 1.8.x and 1.7.x. |
@charris - OK on all except the |
@charris - on the backport, do I need to do anything for that? |
It's probably easier for me to do the backports unless you want the experience. The docstring thing isn't a major point and the masked arrays haven't seen much maintenance in years. If you fixup the rest of the functions in another PR, that would be great and I'll go ahead and put this in when Travis passes. |
In it goes, thanks. If you fix the docstrings it might be worth doing a PEP8 cleanup at the same time. I don't know how conformant the file is at the moment. |
BUG Ensure NotImplemented is passed on in MaskedArray ufunc's
Currently, masked array
ufunc
replacements do not check whether theufunc
call on the data proper actually works (see below). This PR ensures that they pass onNotImplemented
if needed, so that classes more interesting than thestring
below will have a chance to try to do the reverse operation.Note that this is a long-standing bug; I'm not sure how far it can be backported, but hope it can make 1.8.0.
p.s. The reason I stumbled upon this is that I am trying to implement a
MaskedQuantity
class forastropy
[1]. For this package, we already haveUnit
andQuantity
classes, and set it up such thatarray * unit
returns aQuantity
; I'd hope to do havemasked_array * unit
similarly to return aMaskedQuantity
.[1] http://docs.astropy.org/en/latest/units/quantity.html