Skip to content

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

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.

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 8 commits into from
Sep 21, 2017
10 changes: 10 additions & 0 deletions doc/release/1.14.0-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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
Expand Down
67 changes: 55 additions & 12 deletions numpy/ma/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -6184,17 +6184,40 @@ 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
# 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)
Expand All @@ -6206,16 +6229,36 @@ def __str__(self):
return str(masked_print_option._display)

def __repr__(self):
return 'masked'

def flatten(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Inherited from the superclass

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__ = \
Copy link
Member

Choose a reason for hiding this comment

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

So this is OK because the singleton can never be unmasked? Might be cleaner stylewise to just assign the function rather than have all the line continuations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, if the singleton is unmasked there are much bigger problems.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I prefer the continuations, as they ensure that __iop__ doesn't get mispelt as one of the other methods

__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
Expand Down
37 changes: 37 additions & 0 deletions numpy/ma/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -4732,6 +4732,43 @@ 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')

# 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)
Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bit weird how this raises a different error, but shouldn't be a big deal

Copy link
Member Author

Choose a reason for hiding this comment

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

(and importantly, hasn't changed in this PR)

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])
Expand Down