Fix mangled optional_outputs handling

I wasn't being consistent about how I was handling optional_outputs;
this makes it treat both a list and None correctly, and tests that
things get pruned and/or not included in the output struct.

Author: parejkoj

python/lsst/pipe/tasks/calibrateImage.py

@@ -139,16 +139,20 @@ class CalibrateImageConnections(pipeBase.PipelineTaskConnections,
 
     def __init__(self, *, config=None):
         super().__init__(config=config)
-        if not config.optional_outputs:
+        if config.optional_outputs is None or "psf_stars" not in config.optional_outputs:
             del self.psf_stars
+        if config.optional_outputs is None or "psf_stars_footprints" not in config.optional_outputs:
             del self.psf_stars_footprints
+        if config.optional_outputs is None or "astrometry_matches" not in config.optional_outputs:
             del self.astrometry_matches
+        if config.optional_outputs is None or "photometry_matches" not in config.optional_outputs:
             del self.photometry_matches
 
 
 class CalibrateImageConfig(pipeBase.PipelineTaskConfig, pipelineConnections=CalibrateImageConnections):
     optional_outputs = pexConfig.ListField(
-        doc="Which optional outputs to save (as their connection name)?",
+        doc="Which optional outputs to save (as their connection name)?"
+            " If None, do not output any of these datasets.",
         dtype=str,
         # TODO: note somewhere to disable this for benchmarking, but should
         # we always have it on for production runs?
@@ -526,7 +530,7 @@ def run(self, *, exposures, id_generator=None, result=None):
         result.stars = result.stars_footprints.asAstropy()
 
         astrometry_matches, astrometry_meta = self._fit_astrometry(result.exposure, result.stars_footprints)
-        if self.config.optional_outputs:
+        if self.config.optional_outputs is not None and "astrometry_matches" in self.config.optional_outputs:
             result.astrometry_matches = lsst.meas.astrom.denormalizeMatches(astrometry_matches,
                                                                             astrometry_meta)
 
@@ -535,7 +539,7 @@ def run(self, *, exposures, id_generator=None, result=None):
                                                                                result.stars_footprints)
         # fit_photometry returns a new catalog, so we need a new astropy table view.
         result.stars = result.stars_footprints.asAstropy()
-        if self.config.optional_outputs:
+        if self.config.optional_outputs is not None and "photometry_matches" in self.config.optional_outputs:
             result.photometry_matches = lsst.meas.astrom.denormalizeMatches(photometry_matches,
                                                                             photometry_meta)
 

tests/test_calibrateImage.py

@@ -207,6 +207,22 @@ def test_run_2_snaps(self):
 
         self._check_run(calibrate, result)
 
+    def test_run_no_optionals(self):
+        """Test that disabling optional outputs removes them from the output
+        struct, as appropriate.
+        """
+        self.config.optional_outputs = None
+        calibrate = CalibrateImageTask(config=self.config)
+        calibrate.astrometry.setRefObjLoader(self.ref_loader)
+        calibrate.photometry.match.setRefObjLoader(self.ref_loader)
+        result = calibrate.run(exposures=self.exposure)
+
+        self._check_run(calibrate, result)
+        # These are the only optional outputs that require extra computation,
+        # the others are included in the output struct regardless.
+        self.assertNotIn("astrometry_matches", result.getDict())
+        self.assertNotIn("photometry_matches", result.getDict())
+
     def test_handle_snaps(self):
         calibrate = CalibrateImageTask(config=self.config)
         self.assertEqual(calibrate._handle_snaps(self.exposure), self.exposure)
@@ -543,30 +559,39 @@ def test_runQuantum_2_snaps(self):
         self.assertEqual(mock_run.call_args.kwargs.keys(), {"exposures", "result", "id_generator"})
 
     def test_runQuantum_no_optional_outputs(self):
-        config = CalibrateImageTask.ConfigClass()
-        config.optional_outputs = None
-        task = CalibrateImageTask(config=config)
-        lsst.pipe.base.testUtils.assertValidInitOutput(task)
-
-        quantum = lsst.pipe.base.testUtils.makeQuantum(
-            task, self.butler, self.visit_id,
-            {"exposures": [self.exposure0_id],
-             "astrometry_ref_cat": [self.htm_id],
-             "photometry_ref_cat": [self.htm_id],
-             # outputs
-             "exposure": self.visit_id,
-             "stars": self.visit_id,
-             "stars_footprints": self.visit_id,
-             "applied_photo_calib": self.visit_id,
-             "background": self.visit_id,
-             })
-        mock_run = lsst.pipe.base.testUtils.runTestQuantum(task, self.butler, quantum)
-
-        # Ensure the reference loaders have been configured.
-        self.assertEqual(task.astrometry.refObjLoader.name, "gaia_dr3_20230707")
-        self.assertEqual(task.photometry.match.refObjLoader.name, "ps1_pv3_3pi_20170110")
-        # Check that the proper kwargs are passed to run().
-        self.assertEqual(mock_run.call_args.kwargs.keys(), {"exposures", "result", "id_generator"})
+        # All the possible connections: we modify this to test each one by
+        # popping off the removed connection, then re-setting it.
+        connections = {"exposures": [self.exposure0_id, self.exposure1_id],
+                       "astrometry_ref_cat": [self.htm_id],
+                       "photometry_ref_cat": [self.htm_id],
+                       # 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,
+                       "photometry_matches": self.visit_id,
+                       }
+
+        # Check that we can turn off one output at a time.
+        for optional in ["psf_stars", "psf_stars_footprints", "astrometry_matches", "photometry_matches"]:
+            config = CalibrateImageTask.ConfigClass()
+            config.optional_outputs.remove(optional)
+            task = CalibrateImageTask(config=config)
+            lsst.pipe.base.testUtils.assertValidInitOutput(task)
+            # Save the removed one for the next test.
+            temp = connections.pop(optional)
+            # This will fail with "Error in connection ..." if we don't pop
+            # the optional item from the connections list just above.
+            quantum = lsst.pipe.base.testUtils.makeQuantum(task, self.butler, self.visit_id, connections)
+            # This confirms that the outputs did skip the removed one.
+            self.assertNotIn(optional, quantum.outputs)
+            # Restore the one we removed for the next test.
+            connections[optional] = temp
 
     def test_lintConnections(self):
         """Check that the connections are self-consistent.