Skip to content

BUG: Ensure MaskedArray.flat can access single items #4585

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

Merged
merged 2 commits into from
Apr 11, 2014
Merged

BUG: Ensure MaskedArray.flat can access single items #4585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfebb5a
BUG: Ensure MaskedArray.flat can access single items
mhvk Apr 4, 2014
a7eef2d
Ensure single record items also work correctly with MaskedIterator; t…
mhvk Apr 11, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions numpy/ma/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2470,7 +2470,6 @@ def __call__(self, *args, **params):
return result



class MaskedIterator(object):
"""
Flat iterator object to iterate over masked arrays.
Expand Down Expand Up @@ -2534,8 +2533,12 @@ def __getitem__(self, indx):
result = self.dataiter.__getitem__(indx).view(type(self.ma))
if self.maskiter is not None:
_mask = self.maskiter.__getitem__(indx)
_mask.shape = result.shape
result._mask = _mask
if isinstance(_mask, ndarray):
result._mask = _mask
elif isinstance(_mask, np.void):
return mvoid(result, mask=_mask, hardmask=self.ma._hardmask)
elif _mask: # Just a scalar, masked
return masked
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

masked is masked scalar, right? Why is that the right thing here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps a better question is, does isinstance(_mask, ndarray) catch all the alternatives.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried ma.flat.maskiter[1], [:1], and .next() and it would seem isinstance(_mask, ndarray) catches all the alternatives. Of course, I could also put it in a try/except, or turn the if statement around, if isinstance(_mask, np.bool_); perhaps the latter is clearest?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter might be better, it's sort of six of one, half dozen of the other. The underlying type of masked is float64, does that change the previous output type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the advantage of testing for np.bool_ is mostly that it is clearer why one has to do something special. As for previous output type, there was none -- one would get an exception! Of course, not so odd this wasn't caught, as most people would use flat only for the iterator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confess I don't really understand what is going on here, particularly this bit _mask.shape = result.shape. Is this some sort of broadcasting? What is the relation of the two iterators? I could study the code, but the easier route is to ask you ;) Maybe a comment would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not really understand either how the two iterators could return something with a different shape, but worry there is a good reason, but one not necessarily covered by the relatively sparse tests. I checked who last changed the lines with git blame and it seems to be @pierregm - Pierre: this is about MaskedIterator.__getitem__; could you comment?

return result

### This won't work is ravel makes a copy
Expand Down Expand Up @@ -2567,8 +2570,12 @@ def __next__(self):

"""
d = next(self.dataiter)
if self.maskiter is not None and next(self.maskiter):
d = masked
if self.maskiter is not None:
m = next(self.maskiter)
if isinstance(m, np.void):
return mvoid(d, mask=m, hardmask=self.ma._hardmask)
elif m: # Just a scalar, masked
return masked
return d

next = __next__
Expand Down
37 changes: 35 additions & 2 deletions numpy/ma/tests/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1305,17 +1305,50 @@ def test_shrink_mask(self):
assert_equal(a.mask, nomask)

def test_flat(self):
# test simple access
test = masked_array(np.matrix([[1, 2, 3]]), mask=[0, 0, 1])
assert_equal(test.flat[1], 2)
assert_equal(test.flat[2], masked)
self.assertTrue(np.all(test.flat[0:2] == test[0, 0:2]))
# Test flat on masked_matrices
test = masked_array(np.matrix([[1, 2, 3]]), mask=[0, 0, 1])
test.flat = masked_array([3, 2, 1], mask=[1, 0, 0])
control = masked_array(np.matrix([[3, 2, 1]]), mask=[1, 0, 0])
assert_equal(test, control)
#
# Test setting
test = masked_array(np.matrix([[1, 2, 3]]), mask=[0, 0, 1])
testflat = test.flat
testflat[:] = testflat[[2, 1, 0]]
assert_equal(test, control)

testflat[0] = 9
assert_equal(test[0, 0], 9)
# test 2-D record array
# ... on structured array w/ masked records
x = array([[(1, 1.1, 'one'), (2, 2.2, 'two'), (3, 3.3, 'thr')],
[(4, 4.4, 'fou'), (5, 5.5, 'fiv'), (6, 6.6, 'six')]],
dtype=[('a', int), ('b', float), ('c', '|S8')])
x['a'][0, 1] = masked
x['b'][1, 0] = masked
x['c'][0, 2] = masked
x[-1, -1] = masked
xflat = x.flat
assert_equal(xflat[0], x[0, 0])
assert_equal(xflat[1], x[0, 1])
assert_equal(xflat[2], x[0, 2])
assert_equal(xflat[:3], x[0])
assert_equal(xflat[3], x[1, 0])
assert_equal(xflat[4], x[1, 1])
assert_equal(xflat[5], x[1, 2])
assert_equal(xflat[3:], x[1])
assert_equal(xflat[-1], x[-1, -1])
i = 0
j = 0
for xf in xflat:
assert_equal(xf, x[j, i])
i += 1
if i >= x.shape[-1]:
i = 0
j += 1

#------------------------------------------------------------------------------
class TestFillingValues(TestCase):
Expand Down