Merge pull request #9336 from eric-wieser/masked-constant

BUG: Fix various problems with the np.ma.masked constant

3 files changed, 102 insertions, 12 deletions

diff --git a/doc/release/1.14.0-notes.rst b/doc/release/1.14.0-notes.rst
index 6951c6272..73d2bf6a3 100644
--- a/doc/release/1.14.0-notes.rst
+++ b/doc/release/1.14.0-notes.rst
@@ -72,6 +72,16 @@ Previously ``np.tensordot`` raised a ValueError when contracting over 0-length
 dimension. Now it returns a zero array, which is consistent with the behaviour
 of ``np.dot`` and ``np.einsum``.
 
+``np.ma.masked`` is no longer writeable
+---------------------------------------
+Attempts to mutate the ``masked`` constant now error, as the underlying arrays
+are marked readonly. In the past, it was possible to get away with::
+
+    # emulating a function that sometimes returns np.ma.masked
+    val = random.choice([np.ma.masked, 10])
+    var_arr = np.asarray(val)
+    val_arr += 1  # now errors, previously changed np.ma.masked.data
+
 ``np.ma`` functions producing ``fill_value``s have changed
 ----------------------------------------------------------
 Previously, ``np.ma.default_fill_value`` would return a 0d array, but
diff --git a/numpy/ma/core.py b/numpy/ma/core.py
index d8d3ae621..eceb81269 100644
--- a/numpy/ma/core.py
+++ b/numpy/ma/core.py
@@ -6180,17 +6180,40 @@ isMA = isMaskedArray  # backward compatibility
 
 
 class MaskedConstant(MaskedArray):
-    # We define the masked singleton as a float for higher precedence.
-    # Note that it can be tricky sometimes w/ type comparison
-    _data = data = np.array(0.)
-    _mask = mask = np.array(True)
-    _baseclass = ndarray
+    # the lone np.ma.masked instance
+    __singleton = None
+
+    def __new__(cls):
+        if cls.__singleton is None:
+            # We define the masked singleton as a float for higher precedence.
+            # Note that it can be tricky sometimes w/ type comparison
+            data = np.array(0.)
+            mask = np.array(True)
+
+            # prevent any modifications
+            data.flags.writeable = False
+            mask.flags.writeable = False
 
-    def __new__(self):
-        return self._data.view(self)
+            # don't fall back on MaskedArray.__new__(MaskedConstant), since
+            # that might confuse it - this way, the construction is entirely
+            # within our control
+            cls.__singleton = MaskedArray(data, mask=mask).view(cls)
+
+        return cls.__singleton
 
     def __array_finalize__(self, obj):
-        return
+        if self.__singleton is None:
+            # this handles the `.view` in __new__, which we want to copy across
+            # properties normally
+            return super(MaskedConstant, self).__array_finalize__(obj)
+        elif self is self.__singleton:
+            # not clear how this can happen, play it safe
+            pass
+        else:
+            # everywhere else, we want to downcast to MaskedArray, to prevent a
+            # duplicate maskedconstant.
+            self.__class__ = MaskedArray
+            MaskedArray.__array_finalize__(self, obj)
 
     def __array_prepare__(self, obj, context=None):
         return self.view(MaskedArray).__array_prepare__(obj, context)
@@ -6202,16 +6225,36 @@ class MaskedConstant(MaskedArray):
         return str(masked_print_option._display)
 
     def __repr__(self):
-        return 'masked'
-
-    def flatten(self):
-        return masked_array([self._data], dtype=float, mask=[True])
+        if self is self.__singleton:
+            return 'masked'
+        else:
+            # it's a subclass, or something is wrong, make it obvious
+            return object.__repr__(self)
 
     def __reduce__(self):
         """Override of MaskedArray's __reduce__.
         """
         return (self.__class__, ())
 
+    # inplace operations have no effect. We have to override them to avoid
+    # trying to modify the readonly data and mask arrays
+    def __iop__(self, other):
+        return self
+    __iadd__ = \
+    __isub__ = \
+    __imul__ = \
+    __ifloordiv__ = \
+    __itruediv__ = \
+    __ipow__ = \
+        __iop__
+    del __iop__  # don't leave this around
+
+    def copy(self, *args, **kwargs):
+        """ Copy is a no-op on the maskedconstant, as it is a scalar """
+        # maskedconstant is a scalar, so copy doesn't need to copy. There's
+        # precedent for this with `np.bool_` scalars.
+        return self
+
 
 masked = masked_singleton = MaskedConstant()
 masked_array = MaskedArray
diff --git a/numpy/ma/tests/test_core.py b/numpy/ma/tests/test_core.py
index 6aa8f3e08..95c41e493 100644
--- a/numpy/ma/tests/test_core.py
+++ b/numpy/ma/tests/test_core.py
@@ -4799,6 +4799,43 @@ class TestMaskedConstant(object):
         assert_(not isinstance(m, np.ma.core.MaskedConstant))
         assert_(m is not np.ma.masked)
 
+    def test_repr(self):
+        # copies should not exist, but if they do, it should be obvious that
+        # something is wrong
+        assert_equal(repr(np.ma.masked), 'masked')
+
+        # create a new instance in a weird way
+        masked2 = np.ma.MaskedArray.__new__(np.ma.core.MaskedConstant)
+        assert_not_equal(repr(masked2), 'masked')
+
+    def test_pickle(self):
+        from io import BytesIO
+        import pickle
+
+        with BytesIO() as f:
+            pickle.dump(np.ma.masked, f)
+            f.seek(0)
+            res = pickle.load(f)
+        assert_(res is np.ma.masked)
+
+    def test_copy(self):
+        # gh-9328
+        # copy is a no-op, like it is with np.True_
+        assert_equal(
+            np.ma.masked.copy() is np.ma.masked,
+            np.True_.copy() is np.True_)
+
+    def test_immutable(self):
+        orig = np.ma.masked
+        assert_raises(np.ma.core.MaskError, operator.setitem, orig, (), 1)
+        assert_raises(ValueError,operator.setitem, orig.data, (), 1)
+        assert_raises(ValueError, operator.setitem, orig.mask, (), False)
+
+        view = np.ma.masked.view(np.ma.MaskedArray)
+        assert_raises(ValueError, operator.setitem, view, (), 1)
+        assert_raises(ValueError, operator.setitem, view.data, (), 1)
+        assert_raises(ValueError, operator.setitem, view.mask, (), False)
+
 
 def test_masked_array():
     a = np.ma.array([0, 1, 2, 3], mask=[0, 0, 1, 0])