-
-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
BUG operations involving MaskedArray with output given do not return output #8416
Conversation
I turned this into a PR to fix the issue that |
@@ -2988,8 +2988,11 @@ def __array_wrap__(self, obj, context=None): | |||
Wraps the numpy array and sets the mask according to context. | |||
|
|||
""" | |||
result = obj.view(type(self)) | |||
result._update_from(self) | |||
if obj is self: # for in-place operations |
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.
Note: probably this is better done in __array_prepare__
, but given that we hopefully soon have __array_func__
to work with, I felt it not quite worth it to try to define that...
numpy/ma/core.py
Outdated
result = result.copy() | ||
if result is not self: | ||
# MHvK: Not sure why this copy is wanted. | ||
result = result.copy() |
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.
I just spent a little bit of effort trying to figure out if this copy had any purpose, but I can't see any. It's not related to the copying of masks discussed in #5580, for example.
Want to just remove it?
Without this commit, after an operation like ``` a = np.ma.MaskedArray([1., 2., 3.]) b = np.add(a, 1., out=a) ``` one would have `b is not a`. With this PR, it is guaranteed that `b is a`.
6640387
to
7927b3c
Compare
@ahaldane - OK, I agree it made little sence so just removed the strange copy (and rebased). Hopefully, tests will still pass. |
Looks good to me otherwise, merging. Thanks @mhvk |
unnecessary since numpygh-8416
EDIT -- turned into a PR.
Fixes an inconsistency found in #8414:
Here,
d is b
should beTrue
. That it isFalse
s suggested__array_wrap__
is not implemented quite correctly, probably missing a special path forif self is obj
-- this PR fixes that.