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 calculation is correct also for multi-op ufunc with scalars #15450
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,6 +5,7 @@ | |||||
coverage. Complete coverage of all numpy functions is done | ||||||
with less detailed tests in test_function_helpers. | ||||||
""" | ||||||
import erfa.ufunc as erfa_ufunc | ||||||
import numpy as np | ||||||
import pytest | ||||||
from numpy.testing import assert_array_equal | ||||||
|
@@ -151,6 +152,27 @@ def test_3op_ufunc(self): | |||||
assert_array_equal(ma_mb.unmasked, expected_data) | ||||||
assert_array_equal(ma_mb.mask, expected_mask) | ||||||
|
||||||
def test_multi_op_ufunc(self): | ||||||
mask = [True, False, False] | ||||||
iy = Masked([2000, 2001, 2002], mask=mask) | ||||||
im = Masked([1, 2, 3], mask=mask) | ||||||
idy = Masked([10, 20, 25], mask=mask) | ||||||
ihr = Masked([11, 12, 13], mask=[False, False, True]) | ||||||
imn = np.array([50, 51, 52]) | ||||||
isc = np.array([12.5, 13.6, 14.7]) | ||||||
result = erfa_ufunc.dtf2d("utc", iy, im, idy, ihr, imn, isc) | ||||||
# Also test scalar | ||||||
result0 = erfa_ufunc.dtf2d("utc", iy[0], im[0], idy[0], ihr[0], imn[0], isc[0]) | ||||||
expected = erfa_ufunc.dtf2d( | ||||||
"utc", iy.unmasked, im.unmasked, idy.unmasked, ihr.unmasked, imn, isc | ||||||
) | ||||||
expected_mask = np.array([True, False, True]) | ||||||
for res, res0, exp in zip(result, result0, expected): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't these different lengths? So it's not iterating over all of result? As in won't this fail?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is OK: all of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But then zipping them together means the zip only goes to end of the shortest iterator, which is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, result0 is a tuple with 3 entries, it is the same as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah! Good. So then each element of |
||||||
assert_array_equal(res.unmasked, exp) | ||||||
assert_array_equal(res.mask, expected_mask) | ||||||
assert res0.unmasked == exp[0] | ||||||
assert res0.mask == expected_mask[0] | ||||||
|
||||||
@pytest.mark.parametrize("axis", (0, 1, None)) | ||||||
def test_add_reduce(self, axis): | ||||||
ma_reduce = np.add.reduce(self.ma, axis=axis) | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,2 @@ | ||||||||||
Ufuncs with more than 2 operands (such as ``erfa.dtf2d``) now work | ||||||||||
also if all inputs are scalars and more than two inputs have masks. | ||||||||||
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to change this, but perhaps only if there is also a code change? I think there is no worry about |
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.
Are the later elements ever tested? See comments on the
zip
unpacking below.