Merge pull request #186 from lsst/tickets/DM-30564

DM-30564: Improve task label uniqueness exception message
lsst · Jun 9, 2021 · 19a0755 · 19a0755
2 parents 1bb3bd2 + dd2b4cf
commit 19a0755
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 8 deletions.
diff --git a/python/lsst/pipe/base/pipelineIR.py b/python/lsst/pipe/base/pipelineIR.py
@@ -619,10 +619,14 @@ def process_args(argument: Union[str, dict]) -> dict:
             if self.instrument is None:
                 self.instrument = tmp_IR.instrument
             elif self.instrument != tmp_IR.instrument and tmp_IR.instrument is not None:
-                raise ValueError("Only one instrument can be declared in a pipeline or it's imports")
-            if accumulate_tasks.keys() & tmp_IR.tasks.keys():
-                raise ValueError("Task labels in the imported pipelines must "
-                                 "be unique")
+                msg = ("Only one instrument can be declared in a pipeline or its imports. "
+                       f"Top level pipeline defines {self.instrument} but {other_pipeline.location} "
+                       f"defines {tmp_IR.instrument}.")
+                raise ValueError(msg)
+            if duplicate_labels := accumulate_tasks.keys() & tmp_IR.tasks.keys():
+                msg = ("Task labels in the imported pipelines must be unique. "
+                       f"These labels appear multiple times: {duplicate_labels}")
+                raise ValueError(msg)
             accumulate_tasks.update(tmp_IR.tasks)
             self.contracts.extend(tmp_IR.contracts)
             # verify that tmp_IR has unique labels for named subset among

diff --git a/tests/test_pipelineIR.py b/tests/test_pipelineIR.py
@@ -131,8 +131,8 @@ def testImportParsing(self):
             - $PIPE_BASE_DIR/tests/testPipeline1.yaml
             - $PIPE_BASE_DIR/tests/testPipeline2.yaml
         """)
-
-        with self.assertRaises(ValueError):
+        # "modA" is the duplicated label, and it should appear in the error.
+        with self.assertRaisesRegex(ValueError, "modA"):
             PipelineIR.from_string(pipeline_str)
 
         # This should pass, as the conflicting task is excluded
@@ -352,14 +352,15 @@ def testImportingInstrument(self):
         pipeline = PipelineIR.from_string(pipeline_str)
         self.assertEqual(pipeline.instrument, "new.instrument")
 
-        # Test that multiple instruments can't be defined
+        # Test that multiple instruments can't be defined,
+        # and that the error message tells you what instruments were found.
         pipeline_str = textwrap.dedent("""
         description: Test Pipeline
         instrument: new.instrument
         imports:
           - location: $PIPE_BASE_DIR/tests/testPipeline1.yaml
         """)
-        with self.assertRaises(ValueError):
+        with self.assertRaisesRegex(ValueError, "new.instrument .* test.instrument."):
             PipelineIR.from_string(pipeline_str)
 
     def testParameterConfigFormatting(self):