Repository navigation
MaskedArrayFutureWarning when assigning to masked column #5060
Description
I get a numpy masked array warning when I assign one masked column to another. The following code:
a = MaskedColumn([1, 2], name='a', mask=[False, True], dtype='i4')
b = MaskedColumn([3, 4], name='b', mask=[True, False], dtype='i8')
t = Table([a, b])
t['a'] = t['b'] * 2
yields
astropy/table/column.py:1098: MaskedArrayFutureWarning: setting an item on a
masked array which has a shared mask will not copy the mask and also change
the original mask array in the future.
Check the NumPy 1.11 release notes for more information.
ma.MaskedArray.__setitem__(self, index, value)
The numpy 1.11 release notes state the following:
Slicing a MaskedArray will return views of both data and mask. Currently the mask is copy-on-write and changes to the mask in the slice do not propagate to the original mask.
(scheduled for 1.12). More details near the bottom of the release notes (too long to copy full).
I'm not sure what this means for the MaskedColumn case. The current table prints as:
a b
--- ---
-- --
8 4
and I wonder if we would see something like
a b
--- ---
6 --
-- 4
with numpy 1.12 (the values update, but the mask doesn't).
I'd be happy to make the required change(s), since such a warning is confusing at least (and should be gotten rid of), but I'm not sure what the impact is, and thus what change is necessary. Perhaps a different type of assignment in column.py:MaskedColumn.__setitem__? The numpy details say:
To silence these warnings and make your code robust against the upcoming changes, you have two options: if you want to keep the current behavior, call masked_view.unshare_mask() before modifying the mask. If you want to get the future behavior early, use masked_view._sharedmask = False. However, note that setting the _sharedmask attribute will break following explicit calls to masked_view.unshare_mask().
Activity
evertrol commentedon Jun 10, 2016
I think #4675 is related.
saimn commentedon Jun 23, 2016
edit: sorry for submitting by mistake
Actually #4675 is not related. But, as I already use Numpy's new behavior for another project, I can give some details:
Currently when creating a MaskedArray from existing numpy arrays,
copy=Falseis used by default, so one expect that changes ona.dataare reflected ondataand the same fora.mask. But it is not the case:a.maskis copied as soon as something is assigned to it (i.e. the mask is unshared). Numpy's next version will fix this, which is great as it will allow to have shared data and mask.And it is already possible to use this new behavior by setting
._sharedmask = Falseon a masked array.Then, I'm not sure how this change should be handled on Astropy's side: when Numpy 1.12 will be out, this will possibly lead to some unexpected changes for users, but it is difficult to prevent this.
taldcroft commentedon Jun 23, 2016
@evertrol @saimn - astropy passes CI tests against numpy dev right now, so I don't believe there will be any serious regressions like the "I wonder if we would see" example from @evertrol. That also matches my understanding of this change, which is just making the new mask be a bona-fide view of the original in cases where the data are also a view. This behavior is what I would have expected and I did not even realize that the mask is a copy.
I haven't thought about this very deeply, but my feeling is that there is nothing that astropy can or should do about this behavior change besides silencing the warning. In this case astropy is just riding on top of numpy and inherits the behavior. If users have problems then they will need to take the steps suggested in the numpy release notes.
mhvk commentedon Dec 1, 2016
Since this warning came up again in #2997, I think we may want to look slightly closer at our code, just to ensure masks do get copied when they need to. And perhaps to silence the warning in the interim.
saimn commentedon Dec 1, 2016
Also the change was delayed from 1.12 to 1.13 (ref numpy/numpy#5580, thanks @mhvk for pointing it, that's the one I was looking for ;-)). So indeed the warning could be silenced.
saimn commentedon May 23, 2018
This one can probably be closed now ? @taldcroft @mhvk