Merge pull request #2742 from sfinkens/fix-time-attrs

Fix nominal end time in AHI HSD

satpy/readers/ahi_hsd.py

@@ -414,7 +414,7 @@ def start_time(self):
     @property
     def end_time(self):
         """Get the nominal end time."""
-        return self.nominal_start_time
+        return self.nominal_end_time
 
     @property
     def observation_start_time(self):
@@ -426,53 +426,21 @@ def observation_end_time(self):
         """Get the observation end time."""
         return datetime(1858, 11, 17) + timedelta(days=float(self.basic_info["observation_end_time"].item()))
 
+    @property
+    def _timeline(self):
+        return "{:04d}".format(self.basic_info["observation_timeline"][0])
+
     @property
     def nominal_start_time(self):
         """Time this band was nominally to be recorded."""
-        return self._modify_observation_time_for_nominal(self.observation_start_time)
+        calc = _NominalTimeCalculator(self._timeline, self.observation_area)
+        return calc.get_nominal_start_time(self.observation_start_time)
 
     @property
     def nominal_end_time(self):
         """Get the nominal end time."""
-        return self._modify_observation_time_for_nominal(self.observation_end_time)
-
-    @staticmethod
-    def _is_valid_timeline(timeline):
-        """Check that the `observation_timeline` value is not a fill value."""
-        if int(timeline[:2]) > 23:
-            return False
-        return True
-
-    def _modify_observation_time_for_nominal(self, observation_time):
-        """Round observation time to a nominal time based on known observation frequency.
-
-        AHI observations are split into different sectors including Full Disk
-        (FLDK), Japan (JP) sectors, and smaller regional (R) sectors. Each
-        sector is observed at different frequencies (ex. every 10 minutes,
-        every 2.5 minutes, and every 30 seconds). This method will take the
-        actual observation time and round it to the nearest interval for this
-        sector. So if the observation time is 13:32:48 for the "JP02" sector
-        which is the second Japan observation where every Japan observation is
-        2.5 minutes apart, then the result should be 13:32:30.
-
-        """
-        timeline = "{:04d}".format(self.basic_info["observation_timeline"][0])
-        if not self._is_valid_timeline(timeline):
-            warnings.warn(
-                "Observation timeline is fill value, not rounding observation time.",
-                stacklevel=3
-            )
-            return observation_time
-
-        if self.observation_area == "FLDK":
-            dt = 0
-        else:
-            observation_frequency_seconds = {"JP": 150, "R3": 150, "R4": 30, "R5": 30}[self.observation_area[:2]]
-            dt = observation_frequency_seconds * (int(self.observation_area[2:]) - 1)
-
-        return observation_time.replace(
-            hour=int(timeline[:2]), minute=int(timeline[2:4]) + dt//60,
-            second=dt % 60, microsecond=0)
+        calc = _NominalTimeCalculator(self._timeline, self.observation_area)
+        return calc.get_nominal_end_time(self.nominal_start_time)
 
     def get_dataset(self, key, info):
         """Get the dataset."""
@@ -775,3 +743,96 @@ def _ir_calibrate(self, data):
         c2_ = self._header["calibration"]["c2_rad2tb_conversion"][0]
 
         return (c0_ + c1_ * Te_ + c2_ * Te_ ** 2).clip(0)
+
+
+class _NominalTimeCalculator:
+    """Get time when a scan was nominally to be recorded."""
+
+    def __init__(self, timeline, area):
+        """Initialize the nominal timestamp calculator.
+
+        Args:
+            timeline (str): Observation timeline (four characters HHMM)
+            area (str): Observation area (four characters, e.g. FLDK)
+        """
+        self.timeline = self._parse_timeline(timeline)
+        self.area = area
+
+    def _parse_timeline(self, timeline):
+        try:
+            return datetime.strptime(timeline, "%H%M").time()
+        except ValueError:
+            return None
+
+    def get_nominal_start_time(self, observation_start_time):
+        """Get nominal start time of the scan."""
+        return self._modify_observation_time_for_nominal(observation_start_time)
+
+    def get_nominal_end_time(self, nominal_start_time):
+        """Get nominal end time of the scan."""
+        freq = self._observation_frequency
+        return nominal_start_time + timedelta(minutes=freq // 60,
+                                              seconds=freq % 60)
+
+    def _modify_observation_time_for_nominal(self, observation_time):
+        """Round observation time to a nominal time based on known observation frequency.
+
+        AHI observations are split into different sectors including Full Disk
+        (FLDK), Japan (JP) sectors, and smaller regional (R) sectors. Each
+        sector is observed at different frequencies (ex. every 10 minutes,
+        every 2.5 minutes, and every 30 seconds). This method will take the
+        actual observation time and round it to the nearest interval for this
+        sector. So if the observation time is 13:32:48 for the "JP02" sector
+        which is the second Japan observation where every Japan observation is
+        2.5 minutes apart, then the result should be 13:32:30.
+        """
+        if not self.timeline:
+            warnings.warn(
+                "Observation timeline is fill value, not rounding observation time.",
+                stacklevel=3
+            )
+            return observation_time
+        timeline = self._get_closest_timeline(observation_time)
+        dt = self._get_offset_relative_to_timeline()
+        return timeline + timedelta(minutes=dt//60, seconds=dt % 60)
+
+    def _get_closest_timeline(self, observation_time):
+        """Find the closest timeline for the given observation time.
+
+        Needs to check surrounding days because the observation might start
+        a little bit before the planned time.
+
+        Observation start time: 2022-12-31 23:59
+        Timeline: 0000
+        => Nominal start time: 2023-01-01 00:00
+        """
+        delta_days = [-1, 0, 1]
+        surrounding_dates = [
+            (observation_time + timedelta(days=delta)).date()
+            for delta in delta_days
+        ]
+        timelines = [
+            datetime.combine(date, self.timeline)
+            for date in surrounding_dates
+        ]
+        diffs = [
+            abs((timeline - observation_time))
+            for timeline in timelines
+        ]
+        argmin = np.argmin(diffs)
+        return timelines[argmin]
+
+    def _get_offset_relative_to_timeline(self):
+        if self.area == "FLDK":
+            return 0
+        sector_repeat = int(self.area[2:]) - 1
+        return self._observation_frequency * sector_repeat
+
+    @property
+    def _observation_frequency(self):
+        frequencies = {"FLDK": 600, "JP": 150, "R3": 150, "R4": 30, "R5": 30}
+        area = self.area
+        if area != "FLDK":
+            # e.g. JP01, JP02 etc
+            area = area[:2]
+        return frequencies[area]

satpy/tests/reader_tests/test_ahi_hsd.py

@@ -29,7 +29,7 @@
 import numpy as np
 import pytest
 
-from satpy.readers.ahi_hsd import AHIHSDFileHandler
+from satpy.readers.ahi_hsd import AHIHSDFileHandler, _NominalTimeCalculator
 from satpy.readers.utils import get_geostationary_mask
 from satpy.tests.utils import make_dataid
 
@@ -40,7 +40,7 @@
     "satellite": "Himawari-8",
     "observation_area": "FLDK",
     "observation_start_time": 58413.12523839,
-    "observation_end_time": 58413.12562439,
+    "observation_end_time": 58413.132182834444,
     "observation_timeline": "0300",
 }
 FAKE_DATA_INFO: InfoDict = {
@@ -341,9 +341,9 @@ def test_read_band(self, calibrate, *mocks):
 
             time_params_exp = {
                 "nominal_start_time": datetime(2018, 10, 22, 3, 0, 0, 0),
-                "nominal_end_time": datetime(2018, 10, 22, 3, 0, 0, 0),
+                "nominal_end_time": datetime(2018, 10, 22, 3, 10, 0, 0),
                 "observation_start_time": datetime(2018, 10, 22, 3, 0, 20, 596896),
-                "observation_end_time": datetime(2018, 10, 22, 3, 0, 53, 947296),
+                "observation_end_time": datetime(2018, 10, 22, 3, 10, 20, 596896),
             }
             actual_time_params = im.attrs["time_parameters"]
             for key, value in time_params_exp.items():
@@ -417,30 +417,11 @@ def test_time_properties(self):
         """Test start/end/scheduled time properties."""
         with _fake_hsd_handler() as fh:
             assert fh.start_time == datetime(2018, 10, 22, 3, 0)
-            assert fh.end_time == datetime(2018, 10, 22, 3, 0)
+            assert fh.end_time == datetime(2018, 10, 22, 3, 10)
             assert fh.observation_start_time == datetime(2018, 10, 22, 3, 0, 20, 596896)
-            assert fh.observation_end_time == datetime(2018, 10, 22, 3, 0, 53, 947296)
+            assert fh.observation_end_time == datetime(2018, 10, 22, 3, 10, 20, 596896)
             assert fh.nominal_start_time == datetime(2018, 10, 22, 3, 0, 0, 0)
-            assert fh.nominal_end_time == datetime(2018, 10, 22, 3, 0, 0, 0)
-
-    def test_scanning_frequencies(self):
-        """Test scanning frequencies."""
-        with _fake_hsd_handler() as fh:
-            fh.observation_area = "JP04"
-            assert fh.nominal_start_time == datetime(2018, 10, 22, 3, 7, 30, 0)
-            assert fh.nominal_end_time == datetime(2018, 10, 22, 3, 7, 30, 0)
-            fh.observation_area = "R304"
-            assert fh.nominal_start_time == datetime(2018, 10, 22, 3, 7, 30, 0)
-            assert fh.nominal_end_time == datetime(2018, 10, 22, 3, 7, 30, 0)
-            fh.observation_area = "R420"
-            assert fh.nominal_start_time == datetime(2018, 10, 22, 3, 9, 30, 0)
-            assert fh.nominal_end_time == datetime(2018, 10, 22, 3, 9, 30, 0)
-            fh.observation_area = "R520"
-            assert fh.nominal_start_time == datetime(2018, 10, 22, 3, 9, 30, 0)
-            assert fh.nominal_end_time == datetime(2018, 10, 22, 3, 9, 30, 0)
-            fh.observation_area = "FLDK"
-            assert fh.nominal_start_time == datetime(2018, 10, 22, 3, 0, 0, 0)
-            assert fh.nominal_end_time == datetime(2018, 10, 22, 3, 0, 0, 0)
+            assert fh.nominal_end_time == datetime(2018, 10, 22, 3, 10, 0, 0)
 
     def test_blocklen_error(self, *mocks):
         """Test erraneous blocklength."""
@@ -460,25 +441,6 @@ def test_blocklen_error(self, *mocks):
             with pytest.warns(UserWarning, match=r"Actual .* header size does not match expected"):
                 fh._check_fpos(fp_, fpos, 0, "header 1")
 
-    def test_is_valid_time(self):
-        """Test that valid times are correctly identified."""
-        assert AHIHSDFileHandler._is_valid_timeline(FAKE_BASIC_INFO["observation_timeline"])
-        assert not AHIHSDFileHandler._is_valid_timeline("65526")
-
-    def test_time_rounding(self):
-        """Test rounding of the nominal time."""
-        mocker = mock.MagicMock()
-        in_date = datetime(2020, 1, 1, 12, 0, 0)
-
-        with mock.patch("satpy.readers.ahi_hsd.AHIHSDFileHandler._is_valid_timeline", mocker):
-            with _fake_hsd_handler() as fh:
-                mocker.return_value = True
-                assert fh._modify_observation_time_for_nominal(in_date) == datetime(2020, 1, 1, 3, 0, 0)
-                mocker.return_value = False
-                with pytest.warns(UserWarning,
-                                   match=r"Observation timeline is fill value, not rounding observation time"):
-                    assert fh._modify_observation_time_for_nominal(in_date) == datetime(2020, 1, 1, 12, 0, 0)
-
 
 class TestAHICalibration(unittest.TestCase):
     """Test case for various AHI calibration types."""
@@ -669,3 +631,111 @@ def _create_fake_file_handler(in_fname, filename_info=None, filetype_info=None,
     assert in_fname != fh.filename
     assert str(filename_info["segment"]).zfill(2) == fh.filename[0:2]
     return fh
+
+
+class TestNominalTimeCalculator:
+    """Test case for nominal timestamp computation."""
+
+    @pytest.mark.parametrize(
+        ("timeline", "expected"),
+        [
+            ("0300", datetime(2020, 1, 1, 3, 0, 0)),
+            ("65526", datetime(2020, 1, 1, 12, 0, 0))
+        ]
+    )
+    def test_invalid_timeline(self, timeline, expected):
+        """Test handling of invalid timeline."""
+        calc = _NominalTimeCalculator(timeline, "FLDK")
+        res = calc.get_nominal_start_time(datetime(2020, 1, 1, 12, 0, 0))
+        assert res == expected
+
+    @pytest.mark.parametrize(
+        ("area", "expected"),
+        [
+            (
+                "JP01",
+                {"tstart": datetime(2018, 10, 22, 3, 0, 0),
+                 "tend": datetime(2018, 10, 22, 3, 2, 30)}
+            ),
+            (
+                "JP04",
+                {"tstart": datetime(2018, 10, 22, 3, 7, 30, 0),
+                 "tend": datetime(2018, 10, 22, 3, 10, 0, 0)}
+            ),
+            (
+                "R301",
+                {"tstart": datetime(2018, 10, 22, 3, 0, 0),
+                 "tend": datetime(2018, 10, 22, 3, 2, 30)}
+            ),
+            (
+                "R304",
+                {"tstart": datetime(2018, 10, 22, 3, 7, 30, 0),
+                 "tend": datetime(2018, 10, 22, 3, 10, 0, 0)}
+            ),
+            (
+                "R401",
+                {"tstart": datetime(2018, 10, 22, 3, 0, 0),
+                 "tend": datetime(2018, 10, 22, 3, 0, 30)}
+            ),
+            (
+                "R420",
+                {"tstart": datetime(2018, 10, 22, 3, 9, 30, 0),
+                 "tend": datetime(2018, 10, 22, 3, 10, 0, 0)}
+            ),
+            (
+                "R501",
+                {"tstart": datetime(2018, 10, 22, 3, 0, 0),
+                 "tend": datetime(2018, 10, 22, 3, 0, 30)}
+            ),
+            (
+                "R520",
+                {"tstart": datetime(2018, 10, 22, 3, 9, 30, 0),
+                 "tend": datetime(2018, 10, 22, 3, 10, 0, 0)}
+            ),
+        ]
+    )
+    def test_areas(self, area, expected):
+        """Test nominal timestamps for multiple areas."""
+        obs_start_time = datetime(2018, 10, 22, 3, 0, 20, 596896)
+        calc = _NominalTimeCalculator("0300", area)
+        nom_start_time = calc.get_nominal_start_time(obs_start_time)
+        nom_end_time = calc.get_nominal_end_time(nom_start_time)
+        assert nom_start_time == expected["tstart"]
+        assert nom_end_time == expected["tend"]
+
+    @pytest.mark.parametrize(
+        ("timeline", "obs_start_time", "expected"),
+        [
+            (
+                "2350",
+                datetime(2022, 12, 31, 23, 50, 1),
+                {"tstart": datetime(2022, 12, 31, 23, 50, 0),
+                 "tend": datetime(2023, 1, 1, 0, 0, 0)}
+            ),
+            (
+                "2350",
+                datetime(2022, 12, 31, 23, 49, 59),
+                {"tstart": datetime(2022, 12, 31, 23, 50, 0),
+                 "tend": datetime(2023, 1, 1, 0, 0, 0)}
+            ),
+            (
+                "0000",
+                datetime(2023, 1, 1, 0, 0, 1),
+                {"tstart": datetime(2023, 1, 1, 0, 0, 0),
+                 "tend": datetime(2023, 1, 1, 0, 10, 0)}
+            ),
+            (
+                "0000",
+                datetime(2022, 12, 31, 23, 59, 59),
+                {"tstart": datetime(2023, 1, 1, 0, 0, 0),
+                 "tend": datetime(2023, 1, 1, 0, 10, 0)}
+            ),
+        ]
+    )
+    def test_timelines(self, timeline, obs_start_time, expected):
+        """Test nominal timestamps for multiple timelines."""
+        calc = _NominalTimeCalculator(timeline, "FLDK")
+        nom_start_time = calc.get_nominal_start_time(obs_start_time)
+        nom_end_time = calc.get_nominal_end_time(nom_start_time)
+        assert nom_start_time == expected["tstart"]
+        assert nom_end_time == expected["tend"]