BUG: Revert using __array_ufunc__ for MaskedArray #22016
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rcomer
reviewed
Jul 20, 2022
| result = np.minimum.reduce(a, axis=1) | ||
|
|
||
| assert_array_equal(result, expected) | ||
|
|
There was a problem hiding this comment.
I wonder if it is worth leaving this test in? We know from scipy that it would have passed before #21977. Perhaps it is technically easier to revert the lot though.
There was a problem hiding this comment.
The test fails on 1.23.1 the same way it does on this PR.
========================================================================= FAILURES =========================================================================
____________________________________________________ TestMaskedArrayArithmetic.test_minmax_reduce_axis _____________________________________________________
self = <numpy.ma.tests.test_core.TestMaskedArrayArithmetic object at 0x7f72f772fbe0>
def test_minmax_reduce_axis(self):
# Test np.min/maximum.reduce along an axis for 2D array
import numpy as np
data = [[0, 1, 2, 3, 4, 9], [5, 5, 0, 9, 3, 3]]
mask = [[0, 0, 0, 0, 0, 1], [0, 0, 1, 1, 0, 0]]
a = array(data, mask=mask)
expected = array([0, 3], mask=False)
result = np.minimum.reduce(a, axis=1)
> assert_array_equal(result, expected)
a = masked_array(
data=[[0, 1, 2, 3, 4, --],
[5, 5, --, --, 3, 3]],
mask=[[False, False, False, False, False, True],
[False, False, True, True, False, False]],
fill_value=999999)
data = [[0, 1, 2, 3, 4, 9], [5, 5, 0, 9, 3, 3]]
expected = masked_array(data=[0, 3],
mask=[False, False],
fill_value=999999)
mask = [[0, 0, 0, 0, 0, 1], [0, 0, 1, 1, 0, 0]]
np = <module 'numpy' from '/home/matti/oss/numpy/build/testenv/lib/python3.8/site-packages/numpy/__init__.py'>
result = masked_array(data=[0, 0],
mask=False,
fill_value=999999)
self = <numpy.ma.tests.test_core.TestMaskedArrayArithmetic object at 0x7f72f772fbe0>
numpy/ma/tests/test_core.py:1248:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
numpy/ma/testutils.py:225: in assert_array_equal
assert_array_compare(operator.__eq__, x, y,
err_msg = ''
verbose = True
x = masked_array(data=[0, 0],
mask=False,
fill_value=999999)
y = masked_array(data=[0, 3],
mask=[False, False],
fill_value=999999)
numpy/ma/testutils.py:213: in assert_array_compare
return np.testing.assert_array_compare(comparison,
comparison = <built-in function eq>
err_msg = ''
fill_value = True
header = 'Arrays are not equal'
m = False
verbose = True
x = masked_array(data=[0, 0],
mask=False,
fill_value=999999)
y = masked_array(data=[0, 3],
mask=False,
fill_value=999999)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
args = (<built-in function eq>, array([0, 0]), array([0, 3])), kwds = {'err_msg': '', 'header': 'Arrays are not equal', 'verbose': True}
@wraps(func)
def inner(*args, **kwds):
with self._recreate_cm():
> return func(*args, **kwds)
E AssertionError:
E Arrays are not equal
E
E Mismatched elements: 1 / 2 (50%)
E Max absolute difference: 3
E Max relative difference: 1.
E x: array([0, 0])
E y: array([0, 3])
args = (<built-in function eq>, array([0, 0]), array([0, 3]))
func = <function assert_array_compare at 0x7f730118b310>
kwds = {'err_msg': '', 'header': 'Arrays are not equal', 'verbose': True}
self = <contextlib._GeneratorContextManager object at 0x7f73011891f0>
/usr/lib/python3.8/contextlib.py:75: AssertionError
There was a problem hiding this comment.
Ah OK, so it runs at 1.23.1 but gives wrong numbers. That may be useful info for @mdhaber.
|
Thanks Matti, sorry that this is so tricky beyond our test-suite :/. |
|
I think part of the problem is that some strange (incorrect?) results are now expected behaviour. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts PRs #21977 (which itself was a redo of #10622), #21993 and #21999. Please review the comments to all 4 PRs and also make sure this passes on astropy before re-submitting.
I also re-opened issues #4959 and #15200.
Sorry for the mess, I hope I did the revert correctly.