Skip to content

Commit

Permalink
Merge pull request numpy#5580 from jakirkham/fix_masked_array_views
Browse files Browse the repository at this point in the history
BUG, DEP: Fix masked arrays to properly edit views. ( numpy#5558 )
  • Loading branch information
charris authored Jun 9, 2017
2 parents 18f69fd + 0b060fd commit fa913a8
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 95 deletions.
4 changes: 2 additions & 2 deletions doc/source/reference/maskedarray.generic.rst
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,8 @@ is masked.
When accessing a slice, the output is a masked array whose
:attr:`~MaskedArray.data` attribute is a view of the original data, and whose
mask is either :attr:`nomask` (if there was no invalid entries in the original
array) or a copy of the corresponding slice of the original mask. The copy is
required to avoid propagation of any modification of the mask to the original.
array) or a view of the corresponding slice of the original mask. The view is
required to ensure propagation of any modification of the mask to the original.

>>> x = ma.array([1, 2, 3, 4, 5], mask=[0, 1, 0, 0, 1])
>>> mx = x[:3]
Expand Down
25 changes: 1 addition & 24 deletions numpy/ma/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -3357,8 +3357,6 @@ def __setitem__(self, indx, value):
_mask[indx] = tuple([True] * nbfields)
else:
_mask[indx] = True
if not self._isfield:
self._sharedmask = False
return

# Get the _data part of the new value
Expand All @@ -3374,27 +3372,6 @@ def __setitem__(self, indx, value):
_mask = self._mask = make_mask_none(self.shape, _dtype)
_mask[indx] = mval
elif not self._hardmask:
# Unshare the mask if necessary to avoid propagation
# We want to remove the unshare logic from this place in the
# future. Note that _sharedmask has lots of false positives.
if not self._isfield:
notthree = getattr(sys, 'getrefcount', False) and (sys.getrefcount(_mask) != 3)
if self._sharedmask and not (
# If no one else holds a reference (we have two
# references (_mask and self._mask) -- add one for
# getrefcount) and the array owns its own data
# copying the mask should do nothing.
(not notthree) and _mask.flags.owndata):
# 2016.01.15 -- v1.11.0
warnings.warn(
"setting an item on a masked array which has a shared "
"mask will not copy the mask and also change the "
"original mask array in the future.\n"
"Check the NumPy 1.11 release notes for more "
"information.",
MaskedArrayFutureWarning, stacklevel=2)
self.unshare_mask()
_mask = self._mask
# Set the data, then the mask
_data[indx] = dval
_mask[indx] = mval
Expand Down Expand Up @@ -4659,7 +4636,7 @@ def put(self, indices, values, mode='raise'):
if self._mask is nomask and getmask(values) is nomask:
return

m = getmaskarray(self).copy()
m = getmaskarray(self)

if getmask(values) is nomask:
m.put(indices, False, mode=mode)
Expand Down
17 changes: 14 additions & 3 deletions numpy/ma/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,15 +394,26 @@ def test_copy(self):
y1._data.__array_interface__)
self.assertTrue(y1a.mask is y1.mask)

y2 = array(x1, mask=m)
y2 = array(x1, mask=m3)
self.assertTrue(y2._data.__array_interface__ == x1.__array_interface__)
self.assertTrue(y2._mask.__array_interface__ == m.__array_interface__)
self.assertTrue(y2._mask.__array_interface__ == m3.__array_interface__)
self.assertTrue(y2[2] is masked)
y2[2] = 9
self.assertTrue(y2[2] is not masked)
self.assertTrue(y2._mask.__array_interface__ != m.__array_interface__)
self.assertTrue(y2._mask.__array_interface__ == m3.__array_interface__)
self.assertTrue(allequal(y2.mask, 0))

y2a = array(x1, mask=m, copy=1)
self.assertTrue(y2a._data.__array_interface__ != x1.__array_interface__)
#self.assertTrue( y2a.mask is not m)
self.assertTrue(y2a._mask.__array_interface__ != m.__array_interface__)
self.assertTrue(y2a[2] is masked)
y2a[2] = 9
self.assertTrue(y2a[2] is not masked)
#self.assertTrue( y2a.mask is not m)
self.assertTrue(y2a._mask.__array_interface__ != m.__array_interface__)
self.assertTrue(allequal(y2a.mask, 0))

y3 = array(x1 * 1.0, mask=m)
self.assertTrue(filled(y3).dtype is (x1 * 1.0).dtype)

Expand Down
156 changes: 90 additions & 66 deletions numpy/ma/tests/test_old_ma.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import numpy as np
import numpy.core.umath as umath
import numpy.core.fromnumeric as fromnumeric
from numpy.testing import (
TestCase, run_module_suite, assert_, suppress_warnings)
from numpy.testing import TestCase, run_module_suite, assert_
from numpy.ma.testutils import assert_array_equal
from numpy.ma import (
MaskType, MaskedArray, absolute, add, all, allclose, allequal, alltrue,
Expand Down Expand Up @@ -258,73 +257,98 @@ def test_testCI(self):

def test_testCopySize(self):
# Tests of some subtle points of copying and sizing.
with suppress_warnings() as sup:
sup.filter(
np.ma.core.MaskedArrayFutureWarning,
"setting an item on a masked array which has a "
"shared mask will not copy")

n = [0, 0, 1, 0, 0]
m = make_mask(n)
m2 = make_mask(m)
self.assertTrue(m is m2)
m3 = make_mask(m, copy=1)
self.assertTrue(m is not m3)

x1 = np.arange(5)
y1 = array(x1, mask=m)
self.assertTrue(y1._data is not x1)
self.assertTrue(allequal(x1, y1._data))
self.assertTrue(y1.mask is m)

y1a = array(y1, copy=0)
self.assertTrue(y1a.mask is y1.mask)

y2 = array(x1, mask=m, copy=0)
self.assertTrue(y2.mask is m)
self.assertTrue(y2[2] is masked)
y2[2] = 9
self.assertTrue(y2[2] is not masked)
self.assertTrue(y2.mask is not m)
self.assertTrue(allequal(y2.mask, 0))

y3 = array(x1 * 1.0, mask=m)
self.assertTrue(filled(y3).dtype is (x1 * 1.0).dtype)

x4 = arange(4)
x4[2] = masked
y4 = resize(x4, (8,))
self.assertTrue(eq(concatenate([x4, x4]), y4))
self.assertTrue(eq(getmask(y4), [0, 0, 1, 0, 0, 0, 1, 0]))
y5 = repeat(x4, (2, 2, 2, 2), axis=0)
self.assertTrue(eq(y5, [0, 0, 1, 1, 2, 2, 3, 3]))
y6 = repeat(x4, 2, axis=0)
self.assertTrue(eq(y5, y6))
n = [0, 0, 1, 0, 0]
m = make_mask(n)
m2 = make_mask(m)
self.assertTrue(m is m2)
m3 = make_mask(m, copy=1)
self.assertTrue(m is not m3)

x1 = np.arange(5)
y1 = array(x1, mask=m)
self.assertTrue(y1._data is not x1)
self.assertTrue(allequal(x1, y1._data))
self.assertTrue(y1.mask is m)

y1a = array(y1, copy=0)
self.assertTrue(y1a.mask is y1.mask)

y2 = array(x1, mask=m3, copy=0)
self.assertTrue(y2.mask is m3)
self.assertTrue(y2[2] is masked)
y2[2] = 9
self.assertTrue(y2[2] is not masked)
self.assertTrue(y2.mask is m3)
self.assertTrue(allequal(y2.mask, 0))

y2a = array(x1, mask=m, copy=1)
self.assertTrue(y2a.mask is not m)
self.assertTrue(y2a[2] is masked)
y2a[2] = 9
self.assertTrue(y2a[2] is not masked)
self.assertTrue(y2a.mask is not m)
self.assertTrue(allequal(y2a.mask, 0))

y3 = array(x1 * 1.0, mask=m)
self.assertTrue(filled(y3).dtype is (x1 * 1.0).dtype)

x4 = arange(4)
x4[2] = masked
y4 = resize(x4, (8,))
self.assertTrue(eq(concatenate([x4, x4]), y4))
self.assertTrue(eq(getmask(y4), [0, 0, 1, 0, 0, 0, 1, 0]))
y5 = repeat(x4, (2, 2, 2, 2), axis=0)
self.assertTrue(eq(y5, [0, 0, 1, 1, 2, 2, 3, 3]))
y6 = repeat(x4, 2, axis=0)
self.assertTrue(eq(y5, y6))

def test_testPut(self):
# Test of put
with suppress_warnings() as sup:
sup.filter(
np.ma.core.MaskedArrayFutureWarning,
"setting an item on a masked array which has a "
"shared mask will not copy")
d = arange(5)
n = [0, 0, 0, 1, 1]
m = make_mask(n)
x = array(d, mask=m)
self.assertTrue(x[3] is masked)
self.assertTrue(x[4] is masked)
x[[1, 4]] = [10, 40]
self.assertTrue(x.mask is not m)
self.assertTrue(x[3] is masked)
self.assertTrue(x[4] is not masked)
self.assertTrue(eq(x, [0, 10, 2, -1, 40]))

x = array(d, mask=m)
x.put([0, 1, 2], [-1, 100, 200])
self.assertTrue(eq(x, [-1, 100, 200, 0, 0]))
self.assertTrue(x[3] is masked)
self.assertTrue(x[4] is masked)
d = arange(5)
n = [0, 0, 0, 1, 1]
m = make_mask(n)
m2 = m.copy()
x = array(d, mask=m)
self.assertTrue(x[3] is masked)
self.assertTrue(x[4] is masked)
x[[1, 4]] = [10, 40]
self.assertTrue(x.mask is m)
self.assertTrue(x[3] is masked)
self.assertTrue(x[4] is not masked)
self.assertTrue(eq(x, [0, 10, 2, -1, 40]))

x = array(d, mask=m2, copy=True)
x.put([0, 1, 2], [-1, 100, 200])
self.assertTrue(x.mask is not m2)
self.assertTrue(x[3] is masked)
self.assertTrue(x[4] is masked)
self.assertTrue(eq(x, [-1, 100, 200, 0, 0]))

def test_testPut2(self):
# Test of put
d = arange(5)
x = array(d, mask=[0, 0, 0, 0, 0])
z = array([10, 40], mask=[1, 0])
self.assertTrue(x[2] is not masked)
self.assertTrue(x[3] is not masked)
x[2:4] = z
self.assertTrue(x[2] is masked)
self.assertTrue(x[3] is not masked)
self.assertTrue(eq(x, [0, 1, 10, 40, 4]))

d = arange(5)
x = array(d, mask=[0, 0, 0, 0, 0])
y = x[2:4]
z = array([10, 40], mask=[1, 0])
self.assertTrue(x[2] is not masked)
self.assertTrue(x[3] is not masked)
y[:] = z
self.assertTrue(y[0] is masked)
self.assertTrue(y[1] is not masked)
self.assertTrue(eq(y, [10, 40]))
self.assertTrue(x[2] is masked)
self.assertTrue(x[3] is not masked)
self.assertTrue(eq(x, [0, 1, 10, 40, 4]))

def test_testMaPut(self):
(x, y, a10, m1, m2, xm, ym, z, zm, xf, s) = self.d
Expand Down

0 comments on commit fa913a8

Please sign in to comment.