Skip to content

ma.empty/ones/zeros_like to copy mask properly #3404

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

ma.empty/ones/zeros_like to copy mask properly #3404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5a3457
Force empty/ones/zeros_like() to copy mask
tomyun Jun 5, 2013
0908295
Include ones_like, zeros_like public for import *
tomyun Jun 6, 2013
9543818
Add a regression test for #3145
tomyun Jun 6, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions numpy/ma/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
'maximum_fill_value', 'mean', 'min', 'minimum', 'minimum_fill_value',
'mod', 'multiply', 'mvoid',
'negative', 'nomask', 'nonzero', 'not_equal',
'ones', 'outer', 'outerproduct',
'ones', 'ones_like', 'outer', 'outerproduct',
'power', 'prod', 'product', 'ptp', 'put', 'putmask',
'rank', 'ravel', 'remainder', 'repeat', 'reshape', 'resize',
'right_shift', 'round_', 'round',
Expand All @@ -84,7 +84,7 @@
'swapaxes',
'take', 'tan', 'tanh', 'trace', 'transpose', 'true_divide',
'var', 'where',
'zeros']
'zeros', 'zeros_like']

MaskType = np.bool_
nomask = MaskType(0)
Expand Down Expand Up @@ -7179,10 +7179,11 @@ class _convert2ma:
"""
__doc__ = None
#
def __init__(self, funcname, params=None):
def __init__(self, funcname, params=None, copy=False):
self._func = getattr(np, funcname)
self.__doc__ = self.getdoc()
self._extras = params or {}
self._copy = copy
#
def getdoc(self):
"Return the doc of the function (from the doc of the method)."
Expand All @@ -7208,21 +7209,23 @@ def __call__(self, a, *args, **params):
result.fill_value = _extras.get("fill_value", None)
if "hardmask" in common_params:
result._hardmask = bool(_extras.get("hard_mask", False))
if self._copy:
result._mask = result._mask.copy()
return result

arange = _convert2ma('arange', params=dict(fill_value=None, hardmask=False))
clip = np.clip
diff = np.diff
empty = _convert2ma('empty', params=dict(fill_value=None, hardmask=False))
empty_like = _convert2ma('empty_like')
empty_like = _convert2ma('empty_like', copy=True)
frombuffer = _convert2ma('frombuffer')
fromfunction = _convert2ma('fromfunction')
identity = _convert2ma('identity', params=dict(fill_value=None, hardmask=False))
indices = np.indices
ones = _convert2ma('ones', params=dict(fill_value=None, hardmask=False))
ones_like = np.ones_like
ones_like = _convert2ma('ones_like', copy=True)
squeeze = np.squeeze
zeros = _convert2ma('zeros', params=dict(fill_value=None, hardmask=False))
zeros_like = np.zeros_like
zeros_like = _convert2ma('zeros_like', copy=True)

###############################################################################
12 changes: 12 additions & 0 deletions numpy/ma/tests/test_regression.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,17 @@ def test_ddof_corrcoef(self):
# ddof should not have an effect (it gets cancelled out)
assert_allclose(r0.data, r1.data)

def test_like_mask_copy(self):
"""Issue gh-3145"""
m = [False, False, False, True]
a = np.ma.array([1, 2, 3, 4], mask=m)
a_empty = np.ma.empty_like(a)
a_ones = np.ma.ones_like(a)
a_zeros = np.ma.zeros_like(a)
a.mask = False
assert_array_equal(a_empty.mask, m)
assert_array_equal(a_ones.mask, m)
assert_array_equal(a_zeros.mask, m)
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 says that all the *_like functions return arrays in which everything that was masked in the input, is masked in the output. This seems odd to me. Shouldn't e.g. ones_like give you an array whose contents is all-ones and whose mask is all-False?

Copy link
Author

Choose a reason for hiding this comment

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

That might depend on the design of API. Yet, many users would expect *_like functions return arrays of same shape and mask. In either way, current implementation returns a new array with old mask, which is definitely confusing.

Copy link
Member

Choose a reason for hiding this comment

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

Just to follow up, it looks like everyone on the mailing list prefers the API where the *_like functions return a new array with the mask set to nomask, rather than copying the input array's mask, and that's my intuition as well. Want to update the PR to do that? (Or if you have an argument for why it should be the other way around, bring that up on the list?)

Also Pierre GM made a suggestion for how the _convert2ma function might be changed that you might want to take a look at.


if __name__ == "__main__":
run_module_suite()