Review for DM-3651, pipe_tasks - bookkeeping improvements to makeDiscreteSkyMap

python/lsst/pipe/tasks/makeDiscreteSkyMap.py

@@ -62,21 +62,30 @@ def setDefaults(self):
         self.skyMap.tractOverlap = 0.0
 
 class MakeDiscreteSkyMapRunner(pipeBase.TaskRunner):
-    """Want to run on all the dataRefs at once, not one at a time."""
+    """Run a task with all dataRefs at once, rather than one dataRef at a time.
+
+    Call the run method of the task using two positional arguments:
+    - butler: data butler
+    - dataRefList: list of all dataRefs,
+    """
     @staticmethod
     def getTargetList(parsedCmd):
         return [(parsedCmd.butler, parsedCmd.id.refList)]
 
-    def precall(self, parsedCmd):
-        # We overload to disable writing/checking of schemas and configs.
-        # There's only one SkyMap per rerun anyway, so the config is redundant,
-        # and checking it means we can't overwrite or append to one once we've
-        # written it.
-        return True
-
     def __call__(self, args):
+        """
+        @param args     Arguments for Task.run()
+
+        @return:
+        - None if self.doReturnResults false
+        - A pipe_base Struct containing these fields if self.doReturnResults true:
+            - dataRef: the provided data reference
+            - metadata: task metadata after execution of run
+            - result: result returned by task run, or None if the task fails
+        """
         butler, dataRefList = args
         task = self.TaskClass(config=self.config, log=self.log)
+        result = None # in case the task fails
         if self.doRaise:
             result = task.run(butler, dataRefList)
         else:
@@ -86,9 +95,15 @@ def __call__(self, args):
                 task.log.fatal("Failed: %s" % e)
                 if not isinstance(e, pipeBase.TaskError):
                     traceback.print_exc(file=sys.stderr)
-        task.writeMetadata(butler)
+        for dataRef in dataRefList:
+            task.writeMetadata(dataRef)
+
         if self.doReturnResults:
-            return results
+            return pipeBase.Struct(
+                dataRefList = dataRefList,
+                metadata = task.metadata,
+                result = result,
+            )
 
 class MakeDiscreteSkyMapTask(pipeBase.CmdLineTask):
     """!Make a DiscreteSkyMap in a repository, using the bounding box of a set of calexps.
@@ -175,12 +190,17 @@ def run(self, butler, dataRefList):
         )
 
     def _getConfigName(self):
-        """Return the name of the config dataset
+        """Return None to disable saving config
+
+        There's only one SkyMap per repository, so the config is redundant, and checking it means we can't
+        easily overwrite or append to an existing repository.
         """
-        return "%s_makeDiscreteSkyMap_config" % (self.config.coaddName,)
+        return None
 
     def _getMetadataName(self):
-        """Return the name of the metadata dataset
+        """Return None to disable saving metadata
+
+        The metadata is not interesting, and by not saving it we can eliminate a dataset type.
         """
-        return "%s_makeDiscreteSkyMap_metadata" % (self.config.coaddName,)
+        return None
 

tests/testMakeDiscreteSkyMap.py

@@ -0,0 +1,106 @@
+#!/usr/bin/env python
+
+#
+# LSST Data Management System
+# Copyright 2012 LSST Corporation.
+#
+# This product includes software developed by the
+# LSST Project (http://www.lsst.org/).
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the LSST License Statement and
+# the GNU General Public License along with this program.  If not,
+# see <http://www.lsstcorp.org/LegalNotices/>.
+#
+
+import os.path
+import sys
+import shutil
+
+import unittest
+from lsst.utils import getPackageDir
+import lsst.utils.tests as utilsTests
+from lsst.afw.geom import Extent2I, Box2D
+from lsst.daf.persistence import Butler
+from lsst.pipe.tasks.makeDiscreteSkyMap import MakeDiscreteSkyMapTask, DiscreteSkyMap
+
+class MakeDiscreteSkyMapTestCase(unittest.TestCase):
+    """Test MakeDiscreteSkyMapTask"""
+
+    def setUp(self):
+        self.inPath = os.path.join(getPackageDir("obs_test"), "data", "input")
+        self.outPath = os.path.join(os.path.dirname(__file__), "testMakeDiscreteSkyMapOutput")
+        self.config = MakeDiscreteSkyMapTask.ConfigClass()
+        self.config.doWrite = False # repo has no place to put the data
+
+    def tearDown(self):
+        del self.config
+        if True:
+            shutil.rmtree(self.outPath)
+
+    def testBasics(self):
+        """Test construction of a discrete sky map
+        """
+        butler = Butler(root=self.inPath, outputRoot=self.outPath)
+        coordList = [] # list of sky coords of all corners of all calexp
+        for dataId in (
+            dict(visit=1, filter="g"),
+            dict(visit=2, filter="g"),
+            dict(visit=3, filter="r"),
+        ):
+            rawImage = butler.get("raw", dataId)
+            # fake calexp by simply copying raw data; the task just cares about its bounding box
+            # (which is slightly larger for raw, but that doesn't matter for this test)
+            calexp = rawImage
+            butler.put(calexp, "calexp", dataId)
+            calexpWcs = calexp.getWcs()
+            calexpBoxD = Box2D(calexp.getBBox())
+            coordList += [calexpWcs.pixelToSky(corner) for corner in calexpBoxD.getCorners()]
+
+        # use the calexp to make a sky map
+        retVal = MakeDiscreteSkyMapTask.parseAndRun(
+            args=[self.inPath, "--output", self.outPath, "--id", "filter=g^r"],
+            config = self.config,
+            doReturnResults = True,
+        )
+        self.assertEqual(len(retVal.resultList), 1)
+        skyMap = retVal.resultList[0].result.skyMap
+        self.assertEqual(type(skyMap), DiscreteSkyMap)
+        self.assertEqual(len(skyMap), 1)
+        tractInfo = skyMap[0]
+        self.assertEqual(tractInfo.getId(), 0)
+        self.assertEqual(tractInfo.getNumPatches(), Extent2I(3, 3))
+        tractWcs = tractInfo.getWcs()
+        tractBoxD = Box2D(tractInfo.getBBox())
+        for skyPoint in coordList:
+            self.assertTrue(tractBoxD.contains(tractWcs.skyToPixel(skyPoint)))
+
+#-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
+
+def suite():
+    """Returns a suite containing all the test cases in this module."""
+
+    utilsTests.init()
+
+    suites = []
+    suites += unittest.makeSuite(MakeDiscreteSkyMapTestCase)
+    suites += unittest.makeSuite(utilsTests.MemoryTestCase)
+    return unittest.TestSuite(suites)
+
+def run(shouldExit = False):
+    """Run the tests"""
+    utilsTests.run(suite(), shouldExit)
+
+if __name__ == "__main__":
+    if "--display" in sys.argv:
+        display = True
+    run(True)