-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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
Changes from all commits
7254888
a532242
4ff8d0a
8349967
de29dc5
36f481b
2ecd05f
d4509fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
|
@@ -6206,16 +6229,36 @@ 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 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__ = \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, if the singleton is unmasked there are much bigger problems. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I prefer the continuations, as they ensure that |
||
__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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inherited from the superclass