Skip to content
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

np.MaskedArray.transpose() produces partial views #8506

Closed
eric-wieser opened this issue Jan 20, 2017 · 7 comments
Closed

np.MaskedArray.transpose() produces partial views #8506

eric-wieser opened this issue Jan 20, 2017 · 7 comments
Labels
component: numpy.ma masked arrays

Comments

@eric-wieser
Copy link
Member

eric-wieser commented Jan 20, 2017

>>> src = np.ma.masked_where([0, 1, 0], [1, 1337, 1])
>>> dst = np.ma.masked_where([1, 0, 0, 0, 0], np.zeros(5))
>>> dst_view = dst.transpose()
# check the views look ok
>>> dst
masked_array(data = [-- 0 0 0 0],
             mask = [ True False False False False],
       fill_value = 999999)
>>> dst_view
masked_array(data = [-- 0 0 0 0],
             mask = [ True False False False False],
       fill_value = 999999)

# now write a slice of the view
>>> dst_view[2:5] = src
>>> dst_view
masked_array(data = [-- 0 1 -- 1],
             mask = [ True False False  True False],
       fill_value = 999999)

# but looking back at the original, the data was updated but the mask was not!
>>> dst
masked_array(data = [-- 0 1 1337 1],
             mask = [ True False False False False],
       fill_value = 999999)
@eric-wieser
Copy link
Member Author

Repeating this with [:] instead of .tranpose() has the same effect, but also prints out

Warning (from warnings module):
  File "__main__", line 1
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.

@mhvk
Copy link
Contributor

mhvk commented Jan 20, 2017

This behaviour is as intended originally (see masked array docs), but it is not obvious it is a good idea (see #5558), and hence plans are afoot to change it: #5580. The latter PR is also the reason you are getting FutureWarning. One thing that might be useful for #5580 is to check that your examples would work with that PR (in particular for the transpose, for which you do not get the FutureWarning).

@seberg
Copy link
Member

seberg commented Jan 20, 2017

Its pretty nonsense __setmask__ has a copy argument, but it seems like it probably does not matter. I doubt the mask is ever used as a view, which it should for the methods at least....

Seems like all those changes were not enough yet and there are more things where such a warning/change would be needed. Though, at least it may be ok to do one after another.

@eric-wieser
Copy link
Member Author

This behaviour is as intended originally

Yep, I think the best option here is to duplicate the warning inside transpose

@mhvk
Copy link
Contributor

mhvk commented Jan 21, 2017

We tried quite hard not to give warnings that were superfluous -- this only affects those who would like to write to transposed data, which surely is a minority. So, I don't think we can warn inside transpose...

@eric-wieser
Copy link
Member Author

Perhaps there should be a flag on "partial view"s, that causes a warning to be emitted the first time a write happens

@seberg
Copy link
Member

seberg commented Jan 21, 2017

I am not sure, but it might be possible to set _sharedmask to true. However, it may not actually warn in all cases that will change, but only for assignments.

@eric-wieser eric-wieser added the component: numpy.ma masked arrays label Feb 19, 2017
theodoregoetz pushed a commit to theodoregoetz/numpy that referenced this issue Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: numpy.ma masked arrays
Projects
None yet
Development

No branches or pull requests

3 participants