fixup handle exceptions

WARNING: this will cause trouble with the "rename output" so squash
them together!

python/lsst/pipe/tasks/calibrateImage.py

@@ -441,21 +441,28 @@ def runQuantum(self, butlerQC, inputRefs, outputRefs):
         # Specify the fields that `annotate` needs below, to ensure they
         # exist, even as None.
         result = pipeBase.Struct(exposure=None,
-                                 stars=None,
-                                 psf_stars=None,
+                                 stars_footprints=None,
+                                 psf_stars_footprints=None,
                                  )
         try:
-            self.run(exposures=exposures, result=result)
-        except lsst.pipe.base.RepeatableQuantumError as e:
+            self.run(exposures=exposures, result=result, id_generator=id_generator)
+        except pipeBase.RepeatableQuantumError as e:
+            # TODO: what to do about afw catalogs having null metadata by default?
+            result.psf_stars_footprints.metadata = lsst.daf.base.PropertyList()
+            # And it may not even exist here!
+            # result.stars_footprints.metadata = lsst.daf.base.PropertyList()
             error = pipeBase.AnnotatedPartialOutputsError.annotate(
                 e,
-                exposures=[result.exposure],
-                catalogs=[result.psf_stars, result.stars],
-                task=self
+                self,
+                result.exposure,
+                result.psf_stars_footprints,
+                result.stars_footprints,
+                log=self.log
             )
-            raise error from e
-        finally:
             butlerQC.put(result, outputRefs)
+            raise error from e
+
+        butlerQC.put(result, outputRefs)
 
     @timeMethod
     def run(self, *, exposures, id_generator=None, result=None):
@@ -513,20 +520,24 @@ def run(self, *, exposures, id_generator=None, result=None):
 
         result.exposure = self._handle_snaps(exposures)
 
-        result.psf_stars, result.background, candidates = self._compute_psf(result.exposure)
+        result.psf_stars_footprints, result.background, candidates = self._compute_psf(result.exposure)
+        result.psf_stars = result.psf_stars_footprints.asAstropy()
 
         self._measure_aperture_correction(result.exposure, result.psf_stars)
 
         result.stars_footprints = self._find_stars(result.exposure, result.background, id_generator)
-        result.stars = stars.asAstropy()
+        result.stars = result.stars_footprints.asAstropy()
 
-        astrometry_matches, astrometry_meta = self._fit_astrometry(result.exposure, stars_footprints)
+        astrometry_matches, astrometry_meta = self._fit_astrometry(result.exposure, result.stars_footprints)
         if self.config.optional_outputs:
             result.astrometry_matches = lsst.meas.astrom.denormalizeMatches(astrometry_matches,
                                                                             astrometry_meta)
-        result.stars, photometry_matches, \
+        result.stars_footprints, photometry_matches, \
             photometry_meta, result.applied_photo_calib = self._fit_photometry(result.exposure,
                                                                                result.stars_footprints)
+        # The above returns a new catalog, so we need a new astropy table view.
+        result.stars = result.stars_footprints.asAstropy()
+
         if self.config.optional_outputs:
             result.photometry_matches = lsst.meas.astrom.denormalizeMatches(photometry_matches,
                                                                             photometry_meta)

tests/test_calibrateImage.py

@@ -137,12 +137,20 @@ def _check_run(self, calibrate, result, *, photo_calib):
         # re-estimation during source detection.
         self.assertEqual(len(result.background), 4)
 
+        # Both afw and astropy psf_stars catalogs should be populated.
+        self.assertEqual(result.psf_stars["calib_psf_used"].sum(), 3)
+        self.assertEqual(result.psf_stars_footprints["calib_psf_used"].sum(), 3)
+
         # Check that the summary statistics are reasonable.
         summary = result.exposure.info.getSummaryStats()
         self.assertFloatsAlmostEqual(summary.psfSigma, 2.0, rtol=1e-2)
         self.assertFloatsAlmostEqual(summary.ra, self.sky_center.getRa().asDegrees(), rtol=1e-7)
         self.assertFloatsAlmostEqual(summary.dec, self.sky_center.getDec().asDegrees(), rtol=1e-7)
 
+        # Should have finite sky coordinates in the afw and astropy catalogs.
+        self.assertTrue(np.isfinite(result.stars_footprints["coord_ra"]).all())
+        self.assertTrue(np.isfinite(result.stars["coord_ra"]).all())
+
         # Returned photoCalib should be the applied value, not the ==1 one on the exposure.
         self.assertFloatsAlmostEqual(result.applied_photo_calib.getCalibrationMean(),
                                      photo_calib, rtol=2e-3)
@@ -516,7 +524,7 @@ def test_lintConnections(self):
         lsst.pipe.base.testUtils.lintConnections(Connections)
 
     def test_runQuantum_exception(self):
-        """Test handling of exceptions handling in runQuantum.
+        """Test exception handling in runQuantum.
         """
         task = CalibrateImageTask()
         lsst.pipe.base.testUtils.assertValidInitOutput(task)
@@ -529,8 +537,10 @@ def test_runQuantum_exception(self):
              # outputs
              "exposure": self.visit_id,
              "stars": self.visit_id,
+             "stars_footprints": self.visit_id,
              "background": self.visit_id,
              "psf_stars": self.visit_id,
+             "psf_stars_footprints": self.visit_id,
              "applied_photo_calib": self.visit_id,
              "initial_pvi_background": self.visit_id,
              "astrometry_matches": self.visit_id,
@@ -545,19 +555,17 @@ def test_runQuantum_exception(self):
         ):
             lsst.pipe.base.testUtils.runTestQuantum(task, self.butler, quantum, mockRun=False)
 
-        # A RepeatableQuantumError should write partial outputs.
-        class TestError(lsst.pipe.base.RepeatableQuantumError):
-            @property
-            def metadata(self):
-                return {"something": 12345}
+        # A RepeatableQuantumError should write annotated partial outputs.
+        error = lsst.meas.algorithms.MeasureApCorrError(name="test", nSources=100, ndof=101)
 
-        def mock_run(exposures, result):
+        def mock_run(exposures, result=None, id_generator=None):
             """Mock success through compute_psf, but failure after.
             """
             result.exposure = afwImage.ExposureF(10, 10)
-            result.psf_stars = afwTable.SourceCatalog()
+            result.psf_stars_footprints = afwTable.SourceCatalog()
+            result.psf_stars = afwTable.SourceCatalog().asAstropy()
             result.background = afwMath.BackgroundList()
-            raise TestError(msg)
+            raise error
 
         with (
             mock.patch.object(task, "run", side_effect=mock_run),
@@ -569,19 +577,22 @@ def mock_run(exposures, result):
                                                         self.butler,
                                                         quantum,
                                                         mockRun=False)
-        self.assertIn(msg, "\n".join(cm.output))
 
+        logged = "\n".join(cm.output)
+        self.assertIn("Task failed with only partial outputs: ", logged)
+        self.assertIn("MeasureApCorrError", logged)
+
+        # NOTE: This is an integration test of afw Exposure & SourceCatalog
+        # metadata with the error annotation system in pipe_base.
         # Check that we did get the annotated partial outputs...
         pvi = self.butler.get("initial_pvi", self.visit_id)
-        self.assertEqual(pvi.getMetadata()["failure_message"], msg)
-        self.assertIn("TestError", pvi.getMetadata()["failure_type"])
-        # this one is broken!
-        # self.assertEqual(pvi.getMetadata()["failure_metadata"], msg)
-        stars = self.butler.get("initial_psf_stars_footprints", self.visit_id)
-        self.assertEqual(stars.getMetadata()["failure_message"], msg)
-        self.assertIn("TestError", stars.getMetadata()["failure_type"])
-        # this one is broken!
-        # self.assertEqual(pvi.getMetadata()["failure_metadata"], msg)
+        self.assertIn("Unable to measure aperture correction", pvi.metadata["failure.message"])
+        self.assertIn("MeasureApCorrError", pvi.metadata["failure.type"])
+        self.assertEqual(pvi.metadata["failure.metadata.ndof"], 101)
+        stars = self.butler.get("initial_psf_stars_footprints_detector", self.visit_id)
+        self.assertIn("Unable to measure aperture correction", stars.metadata["failure.message"])
+        self.assertIn("MeasureApCorrError", stars.metadata["failure.type"])
+        self.assertEqual(stars.metadata["failure.metadata.ndof"], 101)
         # ... but not the un-produced outputs.
         with self.assertRaises(FileNotFoundError):
             self.butler.get("initial_stars_footprints_detector", self.visit_id)