Merge branch 'tickets/DM-43416'

doc/lsst.ap.verify/index.rst

@@ -32,7 +32,7 @@ Contributing
 ============
 
 ``lsst.ap.verify`` is developed at https://github.com/lsst/ap_verify.
-You can find Jira issues for this module under the `ap_verify <https://jira.lsstcorp.org/issues/?jql=project%20%3D%20DM%20AND%20component%20%3D%20ap_verify>`_ component.
+You can find Jira issues for this module under the `ap_verify <https://rubinobs.atlassian.net/issues/?jql=project%20%3D%20DM%20AND%20component%20%3D%20ap_verify>`_ component.
 
 .. _lsst.ap.verify-pyapi:
 

pipelines/DECam/ApVerify.yaml

@@ -16,3 +16,8 @@ tasks:
     config:
       doPackageAlerts: True
       alertPackager.doWriteAlerts: True
+contracts:
+  # Contracts removed by excluding apPipe
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)

pipelines/DECam/ApVerifyCalibrate.yaml

@@ -18,6 +18,9 @@ tasks:
 contracts:
   # Must re-declare contracts that cross apPipe and metrics boundary, as
   # these were removed on import.
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)
   # Use of ConnectionsClass for templated fields is a workaround for DM-30210
   - detectAndMeasure.connections.ConnectionsClass(config=detectAndMeasure).diaSources.name ==
       fracDiaSourcesToSciSources.connections.ConnectionsClass(config=fracDiaSourcesToSciSources).diaSources.name

pipelines/HSC/ApVerify.yaml

@@ -16,3 +16,8 @@ tasks:
     config:
       doPackageAlerts: True
       alertPackager.doWriteAlerts: True
+contracts:
+  # Contracts removed by excluding apPipe
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)

pipelines/HSC/ApVerifyCalibrate.yaml

@@ -15,3 +15,8 @@ tasks:
     class: lsst.ap.association.DiaPipelineTask
     config:
       doPackageAlerts: True
+contracts:
+  # Contracts removed by excluding apPipe
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)

pipelines/LSSTCam-imSim/ApVerify.yaml

@@ -16,3 +16,8 @@ tasks:
     config:
       doPackageAlerts: True
       alertPackager.doWriteAlerts: True
+contracts:
+  # Contracts removed by excluding apPipe
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)

pipelines/LSSTCam-imSim/ApVerifyCalibrate.yaml

@@ -18,6 +18,9 @@ tasks:
 contracts:
   # Must re-declare contracts that cross apPipe and metrics boundary, as
   # these were removed on import.
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)
   # Use of ConnectionsClass for templated fields is a workaround for DM-30210
   - detectAndMeasure.connections.ConnectionsClass(config=detectAndMeasure).diaSources.name ==
       fracDiaSourcesToSciSources.connections.ConnectionsClass(config=fracDiaSourcesToSciSources).diaSources.name

pipelines/_ingredients/ApVerify.yaml

@@ -19,3 +19,7 @@ tasks:
       # we have an alternative.
       doPackageAlerts: True
       alertPackager.doWriteAlerts: True
+contracts:
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)

pipelines/_ingredients/ApVerifyCalibrate.yaml

@@ -19,6 +19,9 @@ tasks:
       # we have an alternative.
       doPackageAlerts: True
 contracts:
+  - contract: diaPipe.doConfigureApdb or not totalUnassociatedDiaObjects.doReadMarker
+    msg: "totalUnassociatedDiaObjects.doReadMarker requires diaPipe.doConfigureApdb"
+  - (totalUnassociatedDiaObjects.doReadMarker) or (diaPipe.apdb_config_url == totalUnassociatedDiaObjects.apdb_config_url)
   # Metric inputs must match pipeline outputs
   # Use of ConnectionsClass for templated fields is a workaround for DM-30210
   - detectAndMeasure.connections.ConnectionsClass(config=detectAndMeasure).diaSources.name ==

pipelines/_ingredients/MetricsMisc.yaml

@@ -25,3 +25,6 @@ tasks:
       connections.labelName: diaPipe
   totalUnassociatedDiaObjects:
     class: lsst.ap.association.metrics.TotalUnassociatedDiaObjectsMetricTask
+    config:
+        doReadMarker: False  # Impossible if diaPipe uses new-style config
+        apdb_config_url: parameters.apdb_config

pipelines/_ingredients/MetricsMiscCalibrate.yaml

@@ -26,6 +26,9 @@ tasks:
       connections.labelName: diaPipe
   totalUnassociatedDiaObjects:
     class: lsst.ap.association.metrics.TotalUnassociatedDiaObjectsMetricTask
+    config:
+        doReadMarker: False  # Impossible if diaPipe uses new-style config
+        apdb_config_url: parameters.apdb_config
   numSciSources:
     class: lsst.ip.diffim.metrics.NumberSciSourcesMetricTask
   fracDiaSourcesToSciSources:

python/lsst/ap/verify/pipeline_driver.py

@@ -36,8 +36,7 @@
 import logging
 
 import lsst.ctrl.mpexec.execFixupDataId  # not part of lsst.ctrl.mpexec
-import lsst.ctrl.mpexec.cli.pipetask
-from lsst.ap.pipe.make_apdb import makeApdb
+import lsst.dax.apdb as daxApdb
 
 _LOG = logging.getLogger(__name__)
 
@@ -60,7 +59,7 @@ def __init__(self):
                           help="A custom version of the ap_verify pipeline (e.g., with different metrics). "
                                "Defaults to the ApVerify.yaml within --dataset.")
         self.add_argument("--db", "--db_url", default=None,
-                          help="A location for the AP database, formatted as if for ApdbConfig.db_url. "
+                          help="A location for the AP database, formatted as if for apdb-cli create-sql. "
                                "Defaults to an SQLite file in the --output directory.")
         self.add_argument("--skip-pipeline", action="store_true",
                           help="Do not run the AP pipeline itself. This argument is useful "
@@ -91,7 +90,7 @@ def runApPipeGen3(workspace, parsedCmdLine, processes=1):
     """
     log = _LOG.getChild('runApPipeGen3')
 
-    makeApdb(_getApdbArguments(workspace, parsedCmdLine))
+    _makeApdb(workspace, _getApdbArguments(workspace, parsedCmdLine))
 
     pipelineFile = _getPipelineFile(workspace, parsedCmdLine)
     pipelineArgs = ["pipetask", "--long-log", "run",
@@ -197,8 +196,8 @@ def _getPipelineFile(workspace, parsed):
 
 
 def _getApdbArguments(workspace, parsed):
-    """Return the config options for running make_apdb.py on this workspace,
-    as command-line arguments.
+    """Return the arguments for running apdb-cli create-sql on this workspace,
+    as key-value pairs.
 
     Parameters
     ----------
@@ -210,14 +209,14 @@ def _getApdbArguments(workspace, parsed):
 
     Returns
     -------
-    args : `list` of `str`
-        Command-line arguments calling ``--config`` or ``--config-file``,
-        following the conventions of `sys.argv`.
+    args : mapping [`str`]
+        Arguments to `lsst.dax.apdb.sql.Apdb.init_database`.
     """
     if not parsed.db:
         parsed.db = "sqlite:///" + workspace.dbLocation
 
-    args = ["--config", "db_url=" + parsed.db]
+    args = {"db_url": parsed.db,
+            }
 
     return args
 
@@ -239,14 +238,12 @@ def _getConfigArgumentsGen3(workspace, parsed):
         Command-line arguments calling ``--config`` or ``--config-file``,
         following the conventions of `sys.argv`.
     """
-    # Translate APDB-only arguments to work as a sub-config
-    args = [("diaPipe:apdb." + arg if arg != "--config" else arg)
-            for arg in _getApdbArguments(workspace, parsed)]
-    args.extend([
+    return [
+        # APDB config should have been stored in the workspace.
+        "--config", "parameters:apdb_config=" + workspace.dbConfigLocation,
         # Put output alerts into the workspace.
         "--config", "diaPipe:alertPackager.alertWriteLocation=" + workspace.alertLocation,
-    ])
-    return args
+    ]
 
 
 def _getCollectionArguments(workspace, reuse):
@@ -284,3 +281,17 @@ def _getCollectionArguments(workspace, reuse):
     if reuse and oldRuns:
         args.extend(["--extend-run", "--skip-existing"])
     return args
+
+
+def _makeApdb(workspace, args):
+    """Create an APDB and store its config for future use.
+
+    Parameters
+    ----------
+    workspace : `lsst.ap.verify.workspace.Workspace`
+        A Workspace in which to store the database config.
+    args : mapping [`str`]
+        Arguments to `lsst.dax.apdb.sql.Apdb.init_database`.
+    """
+    config = daxApdb.ApdbSql.init_database(**args)
+    config.save(workspace.dbConfigLocation)

python/lsst/ap/verify/workspace.py

@@ -114,6 +114,15 @@ def dbLocation(self):
         Shall be a pathname to a database suitable for the backend of `Apdb`.
         """
 
+    @property
+    @abc.abstractmethod
+    def dbConfigLocation(self):
+        """The absolute location of the config file for the source association
+        database to be created or updated by the pipeline (`str`, read-only).
+
+        The location is assumed to be a Python (`lsst.pex.config.Config`) file.
+        """
+
     @property
     @abc.abstractmethod
     def alertLocation(self):
@@ -189,6 +198,10 @@ def pipelineDir(self):
     def dbLocation(self):
         return os.path.join(self._location, 'association.db')
 
+    @property
+    def dbConfigLocation(self):
+        return os.path.join(self._location, 'apdb.py')
+
     @property
     def alertLocation(self):
         return os.path.join(self._location, 'alerts')

tests/MockApPipe.yaml

@@ -5,6 +5,7 @@ parameters:
   coaddName: goodSeeing
   # only refcat in ap_verify_testdata
   refcat: gaia
+  apdb_config: dummy_path.yaml
 tasks:
   isr:
     class: lsst.ap.verify.testPipeline.MockIsrTask
@@ -54,6 +55,8 @@ tasks:
     class: lsst.ap.verify.testPipeline.MockDiaPipelineTask
     config:
       doWriteAssociatedSources: True
+      doConfigureApdb: False
+      apdb_config_url: parameters.apdb_config
       connections.coaddName: parameters.coaddName
 contracts:
   # Inputs and outputs must match

tests/test_driver.py

@@ -46,7 +46,7 @@ def patchApPipeGen3(method):
     """
     @functools.wraps(method)
     def wrapper(self, *args, **kwargs):
-        dbPatcher = unittest.mock.patch("lsst.ap.verify.pipeline_driver.makeApdb")
+        dbPatcher = unittest.mock.patch("lsst.ap.verify.pipeline_driver._makeApdb")
         patchedMethod = dbPatcher(method)
         return patchedMethod(self, *args, **kwargs)
     return wrapper
@@ -93,29 +93,24 @@ def testrunApPipeGen3Steps(self):
         self.assertTrue(self.workspace.analysisButler.exists("apdb_marker", id))
         self.assertTrue(self.workspace.analysisButler.exists("goodSeeingDiff_assocDiaSrc", id))
 
-    def _getCmdLineArgs(self, parseAndRunArgs):
-        if parseAndRunArgs[0]:
-            return parseAndRunArgs[0][0]
-        elif "args" in parseAndRunArgs[1]:
-            return parseAndRunArgs[1]["args"]
+    def _getArgs(self, call_args):
+        if call_args.args:
+            return call_args.args[1]
+        elif "args" in call_args.kwargs:
+            return call_args.kwargs["args"]
         else:
-            self.fail("No command-line args passed to parseAndRun!")
+            self.fail(f"No APDB args passed to {call_args}!")
 
     @patchApPipeGen3
     def testrunApPipeGen3WorkspaceDb(self, mockDb):
         """Test that runApPipeGen3 places a database in the workspace location by default.
         """
         pipeline_driver.runApPipeGen3(self.workspace, self.apPipeArgs)
 
-        # Test the call to make_apdb.py
         mockDb.assert_called_once()
-        cmdLineArgs = self._getCmdLineArgs(mockDb.call_args)
-        self.assertIn("db_url=sqlite:///" + self.workspace.dbLocation, cmdLineArgs)
-
-        # Test the call to the AP pipeline
-        id = _getDataIds(self.workspace.analysisButler)[0]
-        apdbConfig = self.workspace.analysisButler.get("apdb_marker", id)
-        self.assertEqual(apdbConfig.db_url, "sqlite:///" + self.workspace.dbLocation)
+        dbArgs = self._getArgs(mockDb.call_args)
+        self.assertIn("db_url", dbArgs)
+        self.assertEqual(dbArgs["db_url"], "sqlite:///" + self.workspace.dbLocation)
 
     @patchApPipeGen3
     def testrunApPipeGen3WorkspaceCustom(self, mockDb):
@@ -124,15 +119,10 @@ def testrunApPipeGen3WorkspaceCustom(self, mockDb):
         self.apPipeArgs.db = "postgresql://somebody@pgdb.misc.org/custom_db"
         pipeline_driver.runApPipeGen3(self.workspace, self.apPipeArgs)
 
-        # Test the call to make_apdb.py
         mockDb.assert_called_once()
-        cmdLineArgs = self._getCmdLineArgs(mockDb.call_args)
-        self.assertIn("db_url=" + self.apPipeArgs.db, cmdLineArgs)
-
-        # Test the call to the AP pipeline
-        id = _getDataIds(self.workspace.analysisButler)[0]
-        apdbConfig = self.workspace.analysisButler.get("apdb_marker", id)
-        self.assertEqual(apdbConfig.db_url, self.apPipeArgs.db)
+        dbArgs = self._getArgs(mockDb.call_args)
+        self.assertIn("db_url", dbArgs)
+        self.assertEqual(dbArgs["db_url"], self.apPipeArgs.db)
 
     def testrunApPipeGen3Reuse(self):
         """Test that runApPipeGen3 does not run the pipeline at all (not even with

tests/test_testPipeline.py

@@ -337,7 +337,8 @@ def testMockTransformDiaSourceCatalogTask(self):
 
     def testMockDiaPipelineTask(self):
         config = MockDiaPipelineTask.ConfigClass()
-        config.apdb.db_url = "testing_only"
+        config.doConfigureApdb = False
+        config.apdb_config_url = "testing_only"
         task = MockDiaPipelineTask(config=config)
         pipelineTests.assertValidInitOutput(task)
         result = task.run(pandas.DataFrame(), pandas.DataFrame(), afwImage.ExposureF(),

tests/test_workspace.py

@@ -106,6 +106,15 @@ def testDatabase(self):
         # Workspace spec allows these to be URIs or paths, whatever the Butler accepts
         self._assertNotInDir(self._testbed.dbLocation, url2pathname(self._testbed.repo))
 
+    def testDbConfig(self):
+        """Verify that a WorkspaceGen3 requests a database config in the target
+        directory, but not in any repository.
+        """
+        root = self._testWorkspace
+        self._assertInDir(self._testbed.dbConfigLocation, root)
+        # Workspace spec allows these to be URIs or paths, whatever the Butler accepts
+        self._assertNotInDir(self._testbed.dbConfigLocation, url2pathname(self._testbed.repo))
+
     def testAlerts(self):
         """Verify that a WorkspaceGen3 requests an alert dump in the target
         directory, but not in any repository.