Repository: numpy/numpy
Pull request: BUG: Fix various problems with the np.ma.masked constant
It turns out that `np.ma.masked` is actually pretty bad at preserving identity or value

This PR attempts to fix that.

Fixes #9328 and #4595

Problem statement:
BUG: MaskedConstant  is mutated despite copying
Copying the masked constant shouldn't be possible, and definitely shouldn't allow this to happen
```python
# ok, this is sane
>>> np.ma.masked[()] = 1
MaskError: Cannot alter the masked element.

# making a copy allows modification - seems reasonable
>>> x = np.ma.masked.copy()
>>> x[()] = 2
>>> x.data
array( 2.)

# But the repr is pretty  confusing
>>> x
masked
>>> x is np.ma.masked
False

# And somehow our data is lost by casting?
>>> x.view(np.ma.MaskedArray)
masked_array(data = 0.0,
             mask = False,
       fill_value = 1e+20)

# Worse yet, np.ma.masked is no longer masked!
>>> np.ma.masked.view(np.ma.MaskedArray)
masked_array(data = 2.0,
             mask = False,
       fill_value = 1e+20)
```

So now we can hide arbitrary values inside a fake masked constant

Hints from issue discussion:

Commit message: BUG: Prevent copies of np.ma.MaskedConstant from being created by .view.

This seems to solve the problem everywhere. It's not clear if this works on PyPy.

Fixes gh-9328

Patch to review:
diff --git a/numpy/ma/core.py b/numpy/ma/core.py
index ab4364706c3e..94613ef2f63e 100644
--- a/numpy/ma/core.py
+++ b/numpy/ma/core.py
@@ -6184,32 +6184,43 @@ def isMaskedArray(x):
 
 
 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
+
+    __singleton = None
 
     def __new__(self):
-        return self._data.view(self)
+        if self.__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)
+            self.__singleton = MaskedArray(data, mask=mask).view(self)
 
-    def __array_finalize__(self, obj):
-        return
+        return self.__singleton
 
-    def __array_prepare__(self, obj, context=None):
-        return self.view(MaskedArray).__array_prepare__(obj, context)
+    def __array_finalize__(self, obj):
+        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
+            self.__array_finalize__(obj)
 
-    def __array_wrap__(self, obj, context=None):
-        return self.view(MaskedArray).__array_wrap__(obj, context)
 
     def __str__(self):
         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 masked:
+            return 'masked'
+        else:
+            # something is wrong, make it obvious
+            return object.__repr__(self)
 
     def __reduce__(self):
         """Override of MaskedArray's __reduce__.
diff --git a/numpy/ma/tests/test_core.py b/numpy/ma/tests/test_core.py
index 707fcd1de9fb..bead29d31817 100644
--- a/numpy/ma/tests/test_core.py
+++ b/numpy/ma/tests/test_core.py
@@ -4732,6 +4732,40 @@ def test_ctor(self):
         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')
+        assert_not_equal(repr(np.ma.masked.copy()), '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_write_to_copy(self):
+        # gh-9328
+        x = np.ma.masked.copy()
+        x[()] = 2
+
+        # write succeeds
+        assert_equal(x.data, 2)
+        assert_equal(x.mask, False)
+
+        # original should be unchanged
+        assert_equal(np.ma.masked.data, 0)
+        assert_equal(np.ma.masked.mask, True)
+
+    def test_no_copies(self):
+        MC = np.ma.core.MaskedConstant
+        assert_(not isinstance(np.ma.masked[...], MC))
+        assert_(not isinstance(np.ma.masked.copy(), MC))
+
 
 def test_masked_array():
     a = np.ma.array([0, 1, 2, 3], mask=[0, 0, 1, 0])


Code review comments:
diff_hunk: @@ -6184,32 +6184,43 @@ def isMaskedArray(x):
 
 
 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
+
+    __singleton = None
 
     def __new__(self):
-        return self._data.view(self)
+        if self.__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)
+            self.__singleton = MaskedArray(data, mask=mask).view(self)
 
-    def __array_finalize__(self, obj):
-        return
+        return self.__singleton
 
-    def __array_prepare__(self, obj, context=None):
-        return self.view(MaskedArray).__array_prepare__(obj, context)
+    def __array_finalize__(self, obj):
+        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
+            self.__array_finalize__(obj)
path: numpy/ma/core.py
text: @author:
I can't see a good alternative here to assigning to `__class__` - unfortunately, `__array_finalize__` asks us to operate in place, rather than return a new object.

Does this work on PyPy, @user1?

@author:
Seems to be alright on `PyPy.js`, with a fake inheritance of `MaskedConstant -> MaskedArray -> int`

@user2:
With my above suggestion, I'd hope this won't be necessary any more.

@author:
Pretty sure we're stuck with this - too much of the `MaskedArray` code calls `.view(type(self))`, and we really don't want to have to special case every single one of those.

An alternative approach would be to allow `__array_finalize__` to have a return value, which replaces `self` - but there's almost certainly code out there that returns something random because it doesn't know what its doing, so that's not safe

@user2:
It would seem `anything.view(MaskedConstant)` is almost certainly an error....  And setting `__class__` is quite awful. But I defer to your judgment - I'm on holidays and worry looking in more detail is not going to be a short process...

(and I agree we cannot change `__array_finalize__`)

@author:
> It would seem anything.view(MaskedConstant) is almost certainly an error

Written that way, yes. The problem is any kind of subclass-capable code that tries to reattach the subclass, perhaps after allocating a new buffer. That shouldn't have to know about `np.ma`, so `np.ma` needs to be able to make `.view(MaskedConstant)` do something sensible