Tests fail with Python 3.11 and Numpy 1.24.1: AssertionError

[s3v-]

Some tests start failing with Python 3.11 and Numpy 1.24.1.
See: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028707

> FAIL: test_cannot_edit_extended_header_in_read_only_mode 
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcfile.py", line 219, in test_cannot_edit_extended_header_in_read_only_mode
>     with self.assertRaisesRegex(ValueError, 'read-only'):
> AssertionError: ValueError not raised
> 
> ======================================================================
> FAIL: test_data_is_not_copied_unnecessarily (tests.test_bzip2mrcfile.Bzip2MrcFileTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcobject.py", line 341, in test_data_is_not_copied_unnecessarily
>     assert self.mrcobject.data is data
> AssertionError
> 
> ======================================================================
> FAIL: test_cannot_edit_extended_header_in_read_only_mode (tests.test_gzipmrcfile.GzipMrcFileTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcfile.py", line 219, in test_cannot_edit_extended_header_in_read_only_mode
>     with self.assertRaisesRegex(ValueError, 'read-only'):
> AssertionError: ValueError not raised
> 
> ======================================================================
> FAIL: test_data_is_not_copied_unnecessarily (tests.test_gzipmrcfile.GzipMrcFileTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcobject.py", line 341, in test_data_is_not_copied_unnecessarily
>     assert self.mrcobject.data is data
> AssertionError
> 
> ======================================================================
> FAIL: test_data_is_not_copied_unnecessarily (tests.test_mrcobject.MrcObjectTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcobject.py", line 341, in test_data_is_not_copied_unnecessarily
>     assert self.mrcobject.data is data
> AssertionError
> 
> ======================================================================
> FAIL: test_data_is_not_copied_unnecessarily (tests.test_mrcinterpreter.MrcInterpreterTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcobject.py", line 341, in test_data_is_not_copied_unnecessarily
>     assert self.mrcobject.data is data
> AssertionError
> 
> ======================================================================
> FAIL: test_cannot_edit_extended_header_in_read_only_mode (tests.test_mrcfile.MrcFileTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcfile.py", line 219, in test_cannot_edit_extended_header_in_read_only_mode
>     with self.assertRaisesRegex(ValueError, 'read-only'):
> AssertionError: ValueError not raised
> 
> ======================================================================
> FAIL: test_data_is_not_copied_unnecessarily (tests.test_mrcfile.MrcFileTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcobject.py", line 341, in test_data_is_not_copied_unnecessarily
>     assert self.mrcobject.data is data
> AssertionError
> 
> ======================================================================
> FAIL: test_cannot_edit_extended_header_in_read_only_mode (tests.test_mrcmemmap.MrcMemmapTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/<<PKGBUILDDIR>>/tests/test_mrcfile.py", line 219, in test_cannot_edit_extended_header_in_read_only_mode
>     with self.assertRaisesRegex(ValueError, 'read-only'):
> AssertionError: ValueError not raised
> 

After changing:

assert self.mrcobject.data is data

with:

assert np.array_equal(self.mrcobject.data, data)

in tests/test_mrcfile.py, all tests pass except "AssertionError: ValueError not raised" caused by a numpy bug (link)

I'm not sure about this change, though.

Kind Regards

[s3v-]

in tests/test_mrcfile.py,

I mean tests/test_mrcobject.py
Sorry

[colinpalmer]

Thanks for pointing this out. My automated tests cover Python 3.11 with numpy 1.23 but I hadn't got as far as including 1.24 yet.

Your suggested change would make the tests pass but those tests would no longer check what they're intended to. In that context (making sure the data hasn't been copied) asserting identity is the correct thing to do, so if those tests are now failing it indicates a problem elsewhere which I'll have to look at.

Thanks for the info about the numpy bug too. Hopefully there'll be a fixed version of numpy soon and we won't have to worry about that one.

[rolandmas]

The tests that checks for identity is:

        self.mrcobject.set_data(data)
        assert self.mrcobject.data is data

Yet the set_data() method does:

        # Copy the data if necessary to ensure correct dtype and C ordering
        new_data = np.asanyarray(data, new_dtype, order='C')

So correct me if I'm wrong, but the test is bound to fail, isn't it?

[colinpalmer]

the test is bound to fail, isn't it?

No, the key here is in the "if necessary" part of the comment. np.asanyarray only copies the data if it is not already in the requested order - or at least, it does in all the numpy versions that mrcfile is currently tested with. From these test failures it looks like the behaviour in numpy 1.24 might have changed.

[rolandmas]

So… do we have a way out? With numpy 1.24.2, the only remaining test failures are those of test_data_is_not_copied_unnecessarily (tested with a locally patched testsuite).

[rolandmas]

As maintainer of the mrcfile package in Debian, I'm tempted to just apply s3v-'s patch, if only to be in time for the release… unless you tell me there's a real problem with that :-)

[colinpalmer]

Well, there's a problem with that in that those tests would no longer be testing what they say they're testing. If you want to patch and release, it would be cleaner to just remove those tests completely. But it's up to you, of course. As long as it's only those "data_is_not_copied_unnecessarily" tests that are failing then there shouldn't be any problems that seriously affect mrcfile's functionality.

[rolandmas]

Thank you. I'll patch these tests out with a very conspicuous comment for now. If you find out the root cause and adapt the tests accordingly, I'll be happy to revert the patch :-)

[rathann]

I'm getting this on Fedora rawhide with numpy-1.24.3. I'll skip those tests for now as suggested above.

[rathann]

I guess the related change in numpy 1.24 might be: numpy/numpy#21925 .

[colinpalmer]

This is now fixed in mrcfile 1.5.0