Changing MaskedArray.squeeze() to never return masked
When using ndarray.squeeze, a view is returned, which means you can do the follow (somewhat-contrived) operation:
def fill_contrived(a): a.squeeze()[...] = 2 return a fill_contrived(np.array([1])) array(2)
However, when tried with a masked array, this can fail, breaking liskov subsitution:
fill_contrived(np.ma.array([1], mask=[True])) MaskError: Cannot alter the masked element.
This fails because squeeze breaks the contract of returning a view, instead deciding sometimes to return masked. There is a patch that fixes this in gh-9432 <https://github.com/numpy/numpy/pull/9432> - however, by necessity it breaks any existing code that uses m_arr.squeeze() is np.ma.masked. Is this too breaking a change? Eric
This sort of change seems very similar to the np.diag() change a few years ago. Are there lessons we could learn from then that we could apply to here? Why would the returned view not be a masked array? Ben Root On Tue, Jul 18, 2017 at 9:37 AM, Eric Wieser <wieser.eric+numpy@gmail.com> wrote:
When using ndarray.squeeze, a view is returned, which means you can do the follow (somewhat-contrived) operation:
def fill_contrived(a): a.squeeze()[...] = 2 return a fill_contrived(np.array([1])) array(2)
However, when tried with a masked array, this can fail, breaking liskov subsitution:
fill_contrived(np.ma.array([1], mask=[True])) MaskError: Cannot alter the masked element.
This fails because squeeze breaks the contract of returning a view, instead deciding sometimes to return masked.
There is a patch that fixes this in gh-9432 <https://github.com/numpy/numpy/pull/9432> - however, by necessity it breaks any existing code that uses m_arr.squeeze() is np.ma.masked.
Is this too breaking a change?
Eric
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
On 07/18/2017 09:52 AM, Benjamin Root wrote:
This sort of change seems very similar to the np.diag() change a few years ago. Are there lessons we could learn from then that we could apply to here?
Why would the returned view not be a masked array?
Ben Root
I am in favor of the proposed change below. I'd like to merge it, but before that I want to make sure I understand your comment. Are you referring to the proposed change to make diag return a view instead of a copy? Note that this has not actually happened yet: https://github.com/numpy/numpy/issues/7661 Also, I think this case is different because it does not change core numpy, rather this is to make the MaskedArray module act more consistently with core numpy. Because of that I think it is much less problematic than the diag changes. Cheers, Allan
On Tue, Jul 18, 2017 at 9:37 AM, Eric Wieser <wieser.eric+numpy@gmail.com> wrote:
When using ndarray.squeeze, a view is returned, which means you can do the follow (somewhat-contrived) operation:
def fill_contrived(a): a.squeeze()[...] = 2 return a fill_contrived(np.array([1])) array(2)
However, when tried with a masked array, this can fail, breaking liskov subsitution:
fill_contrived(np.ma.array([1], mask=[True])) MaskError: Cannot alter the masked element.
This fails because squeeze breaks the contract of returning a view, instead deciding sometimes to return masked.
There is a patch that fixes this in gh-9432 <https://github.com/numpy/numpy/pull/9432> - however, by necessity it breaks any existing code that uses m_arr.squeeze() is np.ma.masked.
Is this too breaking a change?
Eric
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
Yes, that is the change I am thinking of. And yes, it hasn't happened yet. But, it has been set to warn for a few years now, and there was a lot of controversy over it when it was first proposed. That said, I do think the way it was handled made sense, and it is a good model to follow for these types of changes. For all intents and purposes, MaskedArray is "core numpy" for many users. Yes, it has its quirks, but it has been very stable for many years, and users have gotten used to the quirks. While I am all for taking steps to eliminate as many quirks as possible, we need to be mindful of such potentially disruptive changes and give users enough of a heads-up about it. Ben Root On Thu, Aug 10, 2017 at 1:00 PM, Allan Haldane <allanhaldane@gmail.com> wrote:
On 07/18/2017 09:52 AM, Benjamin Root wrote:
This sort of change seems very similar to the np.diag() change a few years ago. Are there lessons we could learn from then that we could apply to here?
Why would the returned view not be a masked array?
Ben Root
I am in favor of the proposed change below.
I'd like to merge it, but before that I want to make sure I understand your comment.
Are you referring to the proposed change to make diag return a view instead of a copy? Note that this has not actually happened yet: https://github.com/numpy/numpy/issues/7661
Also, I think this case is different because it does not change core numpy, rather this is to make the MaskedArray module act more consistently with core numpy. Because of that I think it is much less problematic than the diag changes.
Cheers, Allan
On Tue, Jul 18, 2017 at 9:37 AM, Eric Wieser < wieser.eric+numpy@gmail.com> wrote:
When using ndarray.squeeze, a view is returned, which means you can do the follow (somewhat-contrived) operation:
def fill_contrived(a): a.squeeze()[...] = 2 return a fill_contrived(np.array([1])) array(2)
However, when tried with a masked array, this can fail, breaking liskov subsitution:
fill_contrived(np.ma.array([1], mask=[True])) MaskError: Cannot alter the masked element.
This fails because squeeze breaks the contract of returning a view, instead deciding sometimes to return masked.
There is a patch that fixes this in gh-9432 <https://github.com/numpy/numpy/pull/9432> - however, by necessity it breaks any existing code that uses m_arr.squeeze() is np.ma.masked.
Is this too breaking a change?
Eric
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@python.org https://mail.python.org/mailman/listinfo/numpy-discussion
participants (3)
-
Allan Haldane -
Benjamin Root -
Eric Wieser