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
Ensure mask is properly accounted for in Masked.ptp() #15380
Conversation
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
OK, tests pass except for |
expected = self.ma.max(**kwargs) - self.ma.min(**kwargs) | ||
assert_array_equal(o.unmasked, expected.unmasked) | ||
assert_array_equal(o.mask, expected.mask) | ||
out = np.zeros_like(expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth commenting what each test block is doing
out = np.zeros_like(expected) | |
# testing setting via `out` | |
out = np.zeros_like(expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are lots of tests in the file like this. Still, happy to do it, though perhaps once I rebase #15378? Prefer to avoid running CI again...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is for numpy 2.0 compat, there is no need to backport. Unless this is a real bug beyond that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I agree to follow up on nitpicks in the other PR. Let's get this in to unblock the other one. Thanks!
…380-on-v5.3.x Backport PR #15380 on branch v5.3.x (Ensure mask is properly accounted for in Masked.ptp())
…380-on-v5.0.x Backport PR #15380 on branch v5.0.x (Ensure mask is properly accounted for in Masked.ptp())
Description
This pull request is to address a bug in
Masked.ptp()
, which meant that if any element was masked, the result would be masked too. Now it is ensures that it is the same as.max() - .min()
.Found in #15378.