Skip to content

BUG: Masked object array of ndarrays attaches mask to masked array value #8906

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.

Sign up for GitHub

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

Jump to bottom
Open
eric-wieser opened this issue Apr 6, 2017 · 5 comments
Open

BUG: Masked object array of ndarrays attaches mask to masked array value #8906

eric-wieser opened this issue Apr 6, 2017 · 5 comments
Labels
00 - Bug component: numpy.ma masked arrays

Comments

@eric-wieser
Copy link
Member

eric-wieser commented Apr 6, 2017 •
edited
Loading

See below

>>> m = np.ma.array([None], mask=[False])

>>> m[0] = np.ma.masked; m[0]
masked

>>> m[0] = np.eye(3); m[0]
array([[ 1.,  0.,  0.],
       [ 0.,  1.,  0.],
       [ 0.,  0.,  1.]])

>>> m[0] = np.ma.masked; m[0]
masked_array(data =
 [[-- -- --]
 [-- -- --]
 [-- -- --]],
             mask =
 [[ True  True  True]
 [ True  True  True]
 [ True  True  True]],
       fill_value = 1e+20)   # but that was the same line that we ran above!

The culprit is the # Did we extract a single item? check in numpy/ma/core.py:3192.

I just don't think we have the information needed to actually know whether we got a single item from self.data, which could be of any subclass (?).

Maybe the code should become something like:

# Normalize the index. This is wrong, but being right is really hard (#8276)
if not isinstance(idx, tuple): 
    idx = (idx,)

# append an ellipsis if there was not already one, and remember whether we did
could_be_scalar = Ellipsis not in idx
if could_be_scalar:
    idx = idx + (Ellipsis,)

# guaranteed to be a (possibly 0d) array and not a scalar element
dout = data[idx]

# flatten 0d arrays that were not requested
if could_be_scalar and dout.ndim == 0:
    result = result[()]
return result

#8276 (comment) for the lazy
@eric-wieser
Copy link
Member Author

I think I remember seeing that this is actually intended behaviour in the docs somewhere, but can no longer find where. Anyone else remember this being desirable?

@eric-wieser
Copy link
Member Author

eric-wieser commented Apr 19, 2017 •
edited
Loading

Found it: 1.10 release notes, introduced in 1a4d943 (#5962)

Masked arrays containing objects with arrays
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
For such (rare) masked arrays, getting a single masked item no longer returns a
corrupted masked array, but a fully masked version of the item.

@mhvk: What was the rationale for not just returning masked?

This was referenced Apr 19, 2017
Merged
Merged
@mhvk
Copy link
Contributor

mhvk commented Apr 19, 2017

Yikes, that PR coming back to bite again: as you will have seen, it was an unintended consequence of what was quite a nice simplification of MaskedArray.__getitem__. I'm afraid I do not really recall the reason for what I did in #5962, but from rereading it, it seems to have been important that whatever was returned had a length, and len(np.ma.masked) would fail.

@eric-wieser
Copy link
Member Author

it seems to have been important that whatever was returned had a __len__

This doesn't sound like something that we should guarantee - we don't guarantee anything else about the properties of the returned value - and it seems like if you're using a masked object array, it's your responsibility to handle masked return values.

Would you be in favor of reverting this in #8905, where the original bug is fixed properly?

@eric-wieser
Copy link
Member Author

Still present

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
00 - Bug component: numpy.ma masked arrays
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@eric-wieser @mhvk