From 6756edfad754ed1b65f44e8fe84cf2c9d2f53ce9 Mon Sep 17 00:00:00 2001 From: "Marten H. van Kerkwijk" Date: Tue, 2 Jan 2018 14:40:56 -0500 Subject: [PATCH 1/2] BUG: Ensure __array_finalize__ cannot back-mangle shape --- numpy/ma/core.py | 7 ++++++- numpy/ma/tests/test_core.py | 4 +++- numpy/ma/tests/test_old_ma.py | 6 +++++- numpy/ma/tests/test_regression.py | 10 ++++++++++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/numpy/ma/core.py b/numpy/ma/core.py index fb28fa8e5fde..ba0d58577515 100644 --- a/numpy/ma/core.py +++ b/numpy/ma/core.py @@ -2999,12 +2999,15 @@ def __array_finalize__(self, obj): order = "K" _mask = _mask.astype(_mask_dtype, order) - + else: + # Take a view so shape changes, etc., do not propagate back. + _mask = _mask.view() else: _mask = nomask self._mask = _mask # Finalize the mask + # FIXME: this really doesn't belong here. if self._mask is not nomask: try: self._mask.shape = self.shape @@ -3354,6 +3357,8 @@ def __setattr__(self, attr, value): self._mask.shape = self.shape except (AttributeError, TypeError): pass + elif attr == 'shape' and getmask(self) is not nomask: + self._mask.shape = self.shape def __setmask__(self, mask, copy=False): """ diff --git a/numpy/ma/tests/test_core.py b/numpy/ma/tests/test_core.py index 63703f6cd484..b0d48349dc9c 100644 --- a/numpy/ma/tests/test_core.py +++ b/numpy/ma/tests/test_core.py @@ -352,9 +352,11 @@ def test_copy(self): assert_equal(y1._mask.__array_interface__, m.__array_interface__) y1a = array(y1) + # Default for masked array is not to copy; see gh-10318. assert_(y1a._data.__array_interface__ == y1._data.__array_interface__) - assert_(y1a.mask is y1.mask) + assert_(y1a._mask.__array_interface__ == + y1._mask.__array_interface__) y2 = array(x1, mask=m3) assert_(y2._data.__array_interface__ == x1.__array_interface__) diff --git a/numpy/ma/tests/test_old_ma.py b/numpy/ma/tests/test_old_ma.py index 70eab0edc75f..d7b1e3c18b0e 100644 --- a/numpy/ma/tests/test_old_ma.py +++ b/numpy/ma/tests/test_old_ma.py @@ -273,7 +273,11 @@ def test_testCopySize(self): assert_(y1.mask is m) y1a = array(y1, copy=0) - assert_(y1a.mask is y1.mask) + # For copy=False, one might expect that the array would just + # passed on, i.e., that it would be "is" instead of "==". + # See gh-4043 for discussion. + assert_(y1a._mask.__array_interface__ == + y1._mask.__array_interface__) y2 = array(x1, mask=m3, copy=0) assert_(y2.mask is m3) diff --git a/numpy/ma/tests/test_regression.py b/numpy/ma/tests/test_regression.py index 04e10d9d1a1d..96c418a5121b 100644 --- a/numpy/ma/tests/test_regression.py +++ b/numpy/ma/tests/test_regression.py @@ -74,3 +74,13 @@ def test_ddof_corrcoef(self): r1 = np.ma.corrcoef(x, y, ddof=1) # ddof should not have an effect (it gets cancelled out) assert_allclose(r0.data, r1.data) + + def test_mask_not_backmangled(self): + # See gh-10314. Test case taken from gh-3140. + a = np.ma.MaskedArray([1., 2.], mask=[False, False]) + assert_(a.mask.shape == (2,)) + b = np.tile(a, (2, 1)) + # Check that the above no longer changes a.shape to (1, 2) + assert_(a.mask.shape == (2,)) + assert_(b.shape == (2, 2)) + assert_(b.mask.shape == (2, 2)) From b779baea11beb1092faf591d34fa1907b5f8a18e Mon Sep 17 00:00:00 2001 From: Marten van Kerkwijk Date: Sun, 25 Mar 2018 15:15:52 -0400 Subject: [PATCH 2/2] MAINT: Replace ma.__setattr__ with dtype and shape overrides. Since dtype and shape are properties, this needs a somewhat ugly super construction; see https://bugs.python.org/issue14965 --- numpy/ma/core.py | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/numpy/ma/core.py b/numpy/ma/core.py index ba0d58577515..95e67d1662d0 100644 --- a/numpy/ma/core.py +++ b/numpy/ma/core.py @@ -3007,7 +3007,6 @@ def __array_finalize__(self, obj): self._mask = _mask # Finalize the mask - # FIXME: this really doesn't belong here. if self._mask is not nomask: try: self._mask.shape = self.shape @@ -3347,17 +3346,33 @@ def __setitem__(self, indx, value): _mask[indx] = mindx return - def __setattr__(self, attr, value): - super(MaskedArray, self).__setattr__(attr, value) - if attr == 'dtype' and self._mask is not nomask: - self._mask = self._mask.view(make_mask_descr(value), ndarray) - # Try to reset the shape of the mask (if we don't have a void) - # This raises a ValueError if the dtype change won't work + # Define so that we can overwrite the setter. + @property + def dtype(self): + return super(MaskedArray, self).dtype + + @dtype.setter + def dtype(self, dtype): + super(MaskedArray, type(self)).dtype.__set__(self, dtype) + if self._mask is not nomask: + self._mask = self._mask.view(make_mask_descr(dtype), ndarray) + # Try to reset the shape of the mask (if we don't have a void). + # This raises a ValueError if the dtype change won't work. try: self._mask.shape = self.shape except (AttributeError, TypeError): pass - elif attr == 'shape' and getmask(self) is not nomask: + + @property + def shape(self): + return super(MaskedArray, self).shape + + @shape.setter + def shape(self, shape): + super(MaskedArray, type(self)).shape.__set__(self, shape) + # Cannot use self._mask, since it may not (yet) exist when a + # masked matrix sets the shape. + if getmask(self) is not nomask: self._mask.shape = self.shape def __setmask__(self, mask, copy=False):