DM-44009: Trap case on ingest where end time is before exposure begin time

.github/workflows/do_not_merge.yaml

@@ -26,7 +26,7 @@ jobs:
             'DO NOT MERGE'. Remove this commit from the branch before merging
             or change the commit summary."
 
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: Check requirements.txt for branches
         shell: bash

.github/workflows/lint.yaml

@@ -12,5 +12,5 @@ jobs:
   ruff:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: chartboost/ruff-action@v1

.pre-commit-config.yaml

@@ -1,13 +1,13 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.5.0
+    rev: v4.6.0
     hooks:
       - id: check-yaml
       - id: end-of-file-fixer
       - id: trailing-whitespace
       - id: check-toml
   - repo: https://github.com/psf/black
-    rev: 24.3.0
+    rev: 24.4.2
     hooks:
       - id: black
         # It is recommended to specify the latest version of Python
@@ -22,6 +22,6 @@ repos:
         name: isort (python)
   - repo: https://github.com/astral-sh/ruff-pre-commit
     # Ruff version.
-    rev: v0.3.4
+    rev: v0.4.2
     hooks:
       - id: ruff

pyproject.toml

@@ -79,6 +79,7 @@ select = [
     "N",  # pep8-naming
     "W",  # pycodestyle
     "D",  # pydocstyle
+    "UP",  # pyupgrade
 ]
 extend-select = [
     "RUF100", # Warn about unused noqa

python/lsst/obs/base/_instrument.py

@@ -641,12 +641,25 @@ def makeExposureRecordFromObsInfo(
     else:
         raise RuntimeError(f"Unable to determine where to put group metadata in exposure record: {supported}")
 
+    # In some bad observations, the end time is before the begin time. We
+    # can not let that be ingested as-is because it becomes an unbounded
+    # timespan that will not work correctly with calibration lookups. Instead
+    # force the end time to be the begin time.
+    datetime_end = obsInfo.datetime_end
+    if datetime_end < obsInfo.datetime_begin:
+        datetime_end = obsInfo.datetime_begin
+        _LOG.warning(
+            "Exposure %s:%s has end time before begin time. Forcing it to use the begin time.",
+            obsInfo.instrument,
+            obsInfo.observation_id,
+        )
+
     return dimension.RecordClass(
         instrument=obsInfo.instrument,
         id=obsInfo.exposure_id,
         obs_id=obsInfo.observation_id,
         datetime_begin=obsInfo.datetime_begin,
-        datetime_end=obsInfo.datetime_end,
+        datetime_end=datetime_end,
         exposure_time=obsInfo.exposure_time.to_value("s"),
         # we are not mandating that dark_time be calculable
         dark_time=obsInfo.dark_time.to_value("s") if obsInfo.dark_time is not None else None,

python/lsst/obs/base/_read_curated_calibs.py

@@ -303,7 +303,7 @@ def read_all(
         if len(calib_types) != 1:  # set.add(None) has length 1 so None is OK here.
             raise ValueError(f"Error mixing calib types: {calib_types}")
 
-    no_data = all([v == {} for v in calibration_data.values()])
+    no_data = all(v == {} for v in calibration_data.values())
     if no_data:
         raise RuntimeError("No data to ingest")
 

python/lsst/obs/base/butler_tests.py

@@ -131,7 +131,7 @@ def setUp_butler_get(
 
     def _test_exposure(self, name):
         if self.dataIds[name] is unittest.SkipTest:
-            self.skipTest("Skipping %s as requested" % (inspect.currentframe().f_code.co_name))
+            self.skipTest(f"Skipping {inspect.currentframe().f_code.co_name} as requested")
         exp = self.butler.get(name, self.dataIds[name])
 
         md_component = ".metadata"
@@ -215,7 +215,7 @@ def test_subset_raw(self):
     def test_get_linearizer(self):
         """Test that we can get a linearizer for good detectorIds."""
         if self.butler_get_data.linearizer_type is unittest.SkipTest:
-            self.skipTest("Skipping %s as requested" % (inspect.currentframe().f_code.co_name))
+            self.skipTest(f"Skipping {inspect.currentframe().f_code.co_name} as requested")
 
         camera = self.butler.get("camera")
         for detectorId in self.butler_get_data.good_detectorIds:
@@ -228,7 +228,7 @@ def test_get_linearizer(self):
     def test_get_linearizer_bad_detectorIds(self):
         """Check that bad detectorIds raise."""
         if self.butler_get_data.linearizer_type is unittest.SkipTest:
-            self.skipTest("Skipping %s as requested" % (inspect.currentframe().f_code.co_name))
+            self.skipTest(f"Skipping {inspect.currentframe().f_code.co_name} as requested")
 
         for badccd in self.butler_get_data.bad_detectorIds:
             kwargs = {"detector": badccd}

python/lsst/obs/base/utils.py

@@ -123,7 +123,7 @@ def bboxFromIraf(irafBBoxStr):
     """
     mat = re.search(r"^\[([-\d]+):([-\d]+),([-\d]+):([-\d]+)\]$", irafBBoxStr)
     if not mat:
-        raise RuntimeError('Unable to parse IRAF-style bbox "%s"' % irafBBoxStr)
+        raise RuntimeError(f'Unable to parse IRAF-style bbox "{irafBBoxStr}"')
     x0, x1, y0, y1 = (int(_) for _ in mat.groups())
 
     return geom.BoxI(geom.PointI(x0 - 1, y0 - 1), geom.PointI(x1 - 1, y1 - 1))

python/lsst/obs/base/yamlCamera.py

@@ -237,8 +237,9 @@ def makeTransformDict(nativeSys, transformDict, plateScale):
         knownTransformTypes = ["affine", "radial"]
         if transformType not in knownTransformTypes:
             raise RuntimeError(
-                "Saw unknown transform type for %s: %s (known types are: [%s])"
-                % (key, transform["transformType"], ", ".join(knownTransformTypes))
+                "Saw unknown transform type for {}: {} (known types are: [{}])".format(
+                    key, transform["transformType"], ", ".join(knownTransformTypes)
+                )
             )
 
         if transformType == "affine":
@@ -258,8 +259,9 @@ def makeTransformDict(nativeSys, transformDict, plateScale):
             transform = afwGeom.makeRadialTransform(radialCoeffs)
         else:
             raise RuntimeError(
-                'Impossible condition "%s" is not in: [%s])'
-                % (transform["transformType"], ", ".join(knownTransformTypes))
+                'Impossible condition "{}" is not in: [{}])'.format(
+                    transform["transformType"], ", ".join(knownTransformTypes)
+                )
             )
 
         resMap[cameraGeom.CameraSys(key)] = transform

tests/data/ingest/sidecar_data/dataset_end.json

@@ -0,0 +1 @@
+{"telescope": "Pretend", "instrument": "DummyCam", "location": [-5464492.282102363, -2493000.626631501, 2150943.6513960036], "exposure_id": 3000, "visit_id": 3000, "physical_filter": "dummy_g", "datetime_begin": [2456462.0, 0.06193332175925925], "datetime_end": [2456461.0, 0.6230896990740742], "exposure_time": 30.0, "dark_time": 30.0, "boresight_airmass": 1.072389930512202, "boresight_rotation_angle": 270.0, "boresight_rotation_coord": "sky", "detector_num": 1, "detector_name": "RXX_S01", "detector_unique_name": "0_26", "detector_serial": "076", "detector_group": "0", "detector_exposure_id": 180666823, "object": "STRIPE82L", "temperature": 273.75, "pressure": 621.3000000000001, "relative_humidity": 72.5, "tracking_radec": [320.2499333333333, 1.9444444444444445e-05], "altaz_begin": [157.95888945, 68.80710558], "science_program": "o13015", "observation_type": "science", "observation_id": "DummyDataset_End", "observation_reason": "science", "exposure_group": "3000", "observing_day": 20130617, "observation_counter": 3000, "__CONTENT__": "translated"}

tests/data/ingest/sidecar_data/dataset_end.yaml

@@ -0,0 +1,2 @@
+value: 3000
+name: "dataset_3000"

tests/test_ingest.py

@@ -92,25 +92,22 @@ class RawIngestImpliedIndexTestCase(RawIngestTestCase):
 
 
 class RawIngestEdgeCaseTestCase(unittest.TestCase):
-    """Test ingest using non-standard approaches including failures."""
+    """Test ingest using non-standard approaches including failures.
 
-    @classmethod
-    def setUpClass(cls):
+    Must create a new butler for each test because dimension records are
+    globals.
+    """
+
+    def setUp(self):
         butlerConfig = """
 datastore:
   # Want to ingest real files so can't use in-memory datastore
   cls: lsst.daf.butler.datastores.fileDatastore.FileDatastore
 """
-        cls.root = tempfile.mkdtemp(dir=TESTDIR)
-        cls.creatorButler = butlerTests.makeTestRepo(cls.root, {}, config=Config.fromYaml(butlerConfig))
-        DummyCam().register(cls.creatorButler.registry)
+        self.root = tempfile.mkdtemp(dir=TESTDIR)
+        self.creatorButler = butlerTests.makeTestRepo(self.root, {}, config=Config.fromYaml(butlerConfig))
+        DummyCam().register(self.creatorButler.registry)
 
-    @classmethod
-    def tearDownClass(cls):
-        if cls.root is not None:
-            shutil.rmtree(cls.root, ignore_errors=True)
-
-    def setUp(self):
         self.butler = butlerTests.makeTestCollection(self.creatorButler, uniqueId=self.id())
         self.outputRun = self.butler.run
 
@@ -122,6 +119,10 @@ def setUp(self):
         self.good_file = os.path.join(INGESTDIR, "sidecar_data", "dataset_2.yaml")
         self.bad_instrument_file = os.path.join(TESTDIR, "data", "calexp.fits")
 
+    def tearDown(self):
+        if self.root is not None:
+            shutil.rmtree(self.root, ignore_errors=True)
+
     def testSimpleIngest(self):
         # Use the default per-instrument run for this.
         self.task.run([self.good_file])
@@ -134,6 +135,26 @@ def testSimpleIngest(self):
         datasets = list(self.butler.registry.queryDatasets("raw_dict", collections=self.outputRun))
         self.assertEqual(len(datasets), 2)
 
+    def testTimeStampWarning(self):
+        # Now ingest a dataset which should generate a warning because of
+        # the end time being before the begin time.
+        return
+        files = [os.path.join(INGESTDIR, "sidecar_data", "dataset_end.yaml")]
+        with self.assertLogs("lsst.obs.base._instrument", level="WARNING") as cm:
+            self.task.run(files, run=self.outputRun)
+
+        self.assertIn("has end time before begin time", cm.output[0])
+        records = list(
+            self.butler.registry.queryDimensionRecords(
+                "exposure",
+                where="exposure = exp AND instrument = inst",
+                bind={"exp": 3000, "inst": "DummyCam"},
+            )
+        )
+        record = records[0]
+        timespan = record.timespan
+        self.assertEqual(timespan.begin.isot, timespan.end.isot)
+
     def testExplicitIndex(self):
         files = [os.path.join(INGESTDIR, "indexed_data", "_index.json")]
         self.task.run(files, run=self.outputRun)