-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
API: Make MaskedArray.mask return a view, rather than the underlying mask #10308
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
Conversation
I don't know that I like this: in general, python is fairly consistent in just giving you a direct link, so that you can do tests like Separately, I don't think we should ever suggest to use private attributes. |
The other argument for making this change is it gives us the freedom to swap out the implementation for one that computes the mask on the fly - such as how It seems pretty apparent to me that |
Hmm, looking more at #10270, I see at least the rationale. And I see that |
That this PR updates mostly tests seems to indicate that this will break user code. So I share @mhvk's concern. Also, these are the things that should be discussed on the mailing list before merging, since it's an intentional backwards compat break. |
Probably mostly user tests. Only code relying on the identity of
Agreed - I figured I'd make the PR first to work out exactly what needs to change |
Yeah, good idea - is a useful strategy especially if the PR is not hard. Easier to discuss a concrete rather than an abstract change. |
Something else that is slightly illogical is that this changes only how Or, more particularly, a number of additional tests break if one changes this in
|
I'm not sure I understand how |
Sorry that was not clear: I should have written |
I saw a comment with an example of |
Fixing |
Needs rebasing and changing the release note update from 1.15 to 1.17 |
f9245b4
to
ad0bb85
Compare
…mask This prevents consumers from reshaping the mask in place, which breaks things As a result, `x.mask is x.mask` returns `False`, but this was already true of `x.data is x.data`. May also be related to numpygh-10270
ad0bb85
to
599dbad
Compare
close/reopen |
This prevents consumers from reshaping the mask in place, which breaks things
As a result,
x.mask is x.mask
returnsFalse
, but this was already true ofx.data is x.data
.May also be related to gh-10270
This fixes the following: