Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

When accessing MaskedArray rows, always return an mvoid object #483

Merged
merged 2 commits into from Oct 31, 2012

Conversation

Projects
None yet
3 participants
Contributor

astrofrog commented Oct 11, 2012

The current behavior of MaskedArray when accessing a row object is that if all masks are False, the row is an np.void object, and if any are set to True, it is a np.ma.core.mvoid object. The issue with this is that users can access/modify masks for rows that already have at least one mask set to True, but not for rows that have no masks set. For example:

In [1]: a = ma.array(zip([1,2,3]), mask=[0,1,0], dtype=[('x', int)])

In [2]: a[0].mask
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-7-2221dd3fce8c> in <module>()
----> 1 a[0].mask

AttributeError: 'numpy.void' object has no attribute 'mask'

In [3]: a[1].mask
Out[3]: (True,)

There is no reason why a row should behave differently whether any values are masked or not, so this PR defaults to always returning an mvoid object, otherwise mask values cannot be set for rows that start off with no mask values set.

(Of course, the present implementation in Numpy also has a performance impact - for arrays with many fields, each call to a row has to flatten the mask of the whole record just to check what type to return, which is inefficient. With this PR, row access should be faster for masked arrays. But this is secondary.)

Contributor

WeatherGod commented Oct 11, 2012

As a masked array user, I have always considered the masked records stuff to be somewhat flaky and convoluted. So, you would hear no objections from me on any attempts to simplify the handling of this.

Contributor

astrofrog commented Oct 20, 2012

It seems there are no objections to this - can it be merged in time for 1.7.0?

Contributor

astrofrog commented Oct 31, 2012

@WeatherGod - what do you think? Can this be merged?

Owner

njsmith commented Oct 31, 2012

No-one seems to be objecting, so I'll merge it to mainline, anyway.

Re: 1.7, this is a change where it's hard to know for sure how it will affect people's code, and 1.7 has already gone through several cycles of release-candidate-testing, so I'd rather we leave this for 1.8...

@njsmith njsmith added a commit that referenced this pull request Oct 31, 2012

@njsmith njsmith Merge pull request #483 from astrofrog/fix-masked-getitem
When accessing MaskedArray rows, always return an mvoid object
526b764

@njsmith njsmith merged commit 526b764 into numpy:master Oct 31, 2012

1 check passed

default The Travis build passed
Details
Contributor

astrofrog commented Oct 31, 2012

Thanks! 1.8 is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment