Skip to content
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

satpy/tests/scene_tests/test_resampling.py is using called_once in assertions rather than assert_called_once, causing test failures on Python 3.12 #2646

Closed
ArrayBolt3 opened this issue Nov 22, 2023 · 2 comments · Fixed by #2648

Comments

@ArrayBolt3
Copy link
Contributor

Describe the bug
satpy tests fail with Python 3.12 due to the use of a method "called_once" that is not a valid assertion method. See python/cpython#100690

To Reproduce
Run the satpy tests using Python 3.12.

Expected behavior
No tests should fail.

Actual results

=================================== FAILURES ===================================
_____________ TestSceneResampling.test_resample_reduce_data_toggle _____________

self = <satpy.tests.scene_tests.test_resampling.TestSceneResampling object at 0x7fe3f80d82f0>
rs = <MagicMock name='resample_dataset' id='140615976571808'>

    @mock.patch('satpy.scene.resample_dataset')
    def test_resample_reduce_data_toggle(self, rs):
        """Test that the Scene can be reduced or not reduced during resampling."""
        from pyresample.geometry import AreaDefinition
    
        rs.side_effect = self._fake_resample_dataset_force_20x20
        proj_str = ('+proj=lcc +datum=WGS84 +ellps=WGS84 '
                    '+lon_0=-95. +lat_0=25 +lat_1=25 +units=m +no_defs')
        target_area = AreaDefinition('test', 'test', 'test', proj_str, 4, 4, (-1000., -1500., 1000., 1500.))
        area_def = AreaDefinition('test', 'test', 'test', proj_str, 5, 5, (-1000., -1500., 1000., 1500.))
        area_def.get_area_slices = mock.MagicMock()
        get_area_slices = area_def.get_area_slices
        get_area_slices.return_value = (slice(0, 3, None), slice(0, 3, None))
        area_def_big = AreaDefinition('test', 'test', 'test', proj_str, 10, 10, (-1000., -1500., 1000., 1500.))
        area_def_big.get_area_slices = mock.MagicMock()
        get_area_slices_big = area_def_big.get_area_slices
        get_area_slices_big.return_value = (slice(0, 6, None), slice(0, 6, None))
    
        # Test that data reduction can be disabled
        scene = Scene(filenames=['fake1_1.txt'], reader='fake1')
        scene.load(['comp19'])
        scene['comp19'].attrs['area'] = area_def
        scene['comp19_big'] = xr.DataArray(
            da.zeros((10, 10)), dims=('y', 'x'),
            attrs=scene['comp19'].attrs.copy())
        scene['comp19_big'].attrs['area'] = area_def_big
        scene['comp19_copy'] = scene['comp19'].copy()
        orig_slice_data = scene._slice_data
        # we force the below order of processing to test that success isn't
        # based on data of the same resolution being processed together
        test_order = [
            make_cid(**scene['comp19'].attrs),
            make_cid(**scene['comp19_big'].attrs),
            make_cid(**scene['comp19_copy'].attrs),
        ]
        with mock.patch('satpy.scene.Scene._slice_data') as slice_data, \
                mock.patch('satpy.dataset.dataset_walker') as ds_walker:
            ds_walker.return_value = test_order
            slice_data.side_effect = orig_slice_data
            scene.resample(target_area, reduce_data=False)
            assert not slice_data.called
            assert not get_area_slices.called
            scene.resample(target_area)
>           assert slice_data.called_once

satpy/tests/scene_tests/test_resampling.py:322: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <MagicMock name='_slice_data' id='140615696334768'>, name = 'called_once'

    def __getattr__(self, name):
        if name in {'_mock_methods', '_mock_unsafe'}:
            raise AttributeError(name)
        elif self._mock_methods is not None:
            if name not in self._mock_methods or name in _all_magics:
                raise AttributeError("Mock object has no attribute %r" % name)
        elif _is_magic(name):
            raise AttributeError(name)
        if not self._mock_unsafe and (not self._mock_methods or name not in self._mock_methods):
            if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')) or name in _ATTRIB_DENY_LIST:
>               raise AttributeError(
                    f"{name!r} is not a valid assertion. Use a spec "
                    f"for the mock if {name!r} is meant to be an attribute.")
E               AttributeError: 'called_once' is not a valid assertion. Use a spec for the mock if 'called_once' is meant to be an attribute.

/usr/lib/python3.12/unittest/mock.py:663: AttributeError

Environment Info:

  • OS: Ubuntu 24.04 Development Version
  • Satpy Version: 0.44.0
  • PyResample Version: 1.27.1

Additional context
This bug can be resolved with the following patch (at least this is how I'm patching it in Ubuntu currently):

--- a/satpy/tests/scene_tests/test_resampling.py
+++ b/satpy/tests/scene_tests/test_resampling.py
@@ -319,14 +319,14 @@ class TestSceneResampling:
             assert not slice_data.called
             assert not get_area_slices.called
             scene.resample(target_area)
-            assert slice_data.called_once
-            assert get_area_slices.called_once
+            assert slice_data.assert_called_once
+            assert get_area_slices.assert_called_once
             scene.resample(target_area, reduce_data=True)
             # 2 times for each dataset
             # once for default (reduce_data=True)
             # once for kwarg forced to `True`
             assert slice_data.call_count == 2 * 3 
-            assert get_area_slices.called_once
+            assert get_area_slices.assert_called_once
 
     def test_resample_ancillary(self):
         """Test that the Scene reducing data does not affect final output."""
@mraspaud
Copy link
Member

@ArrayBolt3 thanks for reporting this issue! Feel free to submit a pull request for it.
I think though that
slice_data.assert_called_once() should be use instead of
assert slice_data.assert_called_once, correct?

@ArrayBolt3
Copy link
Contributor Author

You're right, I just didn't know what I was doing :P Technically the weird "assert assert" thing I did seems to work, but it is unnecessary and weird now that I'm looking closer at it.

I'll try and submit a (corrected) pull request sometime afternoon CST.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants