Skip to content

Commit

Permalink
Clean up storage class conversion tests.
Browse files Browse the repository at this point in the history
This includes:

 - Using TaskMetadata.from_dict instead of to_dict in comparisons to
   make the intent clearer.

 - Documenting what the log-inspection utility method actually tests.

 - Making one test (test_from_pipeline_intermediates_differ)
   registering a dataset type before building a QG because that's
   what's needed in general for correctness (even though it didn't
   matter here).

 - Renaming and re-documenting another test
   (test_from_pipeline_inconsistent_dataset_types) where we were
   actually testing that building the QG and then changing a
   dataset type out from under it can be a problem.
  • Loading branch information
TallJimbo committed Dec 1, 2023
1 parent ae2ae85 commit 1a05997
Showing 1 changed file with 20 additions and 16 deletions.
36 changes: 20 additions & 16 deletions tests/test_simple_pipeline_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,12 @@ def _configure_pipeline(self, config_a_cls, config_b_cls, storageClass_a=None, s
return executor

def _test_logs(self, log_output, input_type_a, output_type_a, input_type_b, output_type_b):
"""Check the expected input types received by tasks A and B"""
"""Check the expected input types received by tasks A and B.
Note that these are the types as seen from the perspective of the task,
so they must be consistent with the task's connections, but may not be
consistent with the registry dataset types.
"""
all_logs = "\n".join(log_output)
self.assertIn(f"lsst.a:Run method given data of type: {input_type_a}", all_logs)
self.assertIn(f"lsst.b:Run method given data of type: {input_type_b}", all_logs)
Expand All @@ -191,12 +196,6 @@ def test_from_pipeline(self):

def test_from_pipeline_intermediates_differ(self):
"""Run pipeline but intermediates definition in registry differs."""
executor = self._configure_pipeline(
NoDimensionsTestTask.ConfigClass,
NoDimensionsTestTask.ConfigClass,
storageClass_b="TaskMetadataLike",
)

# Pre-define the "intermediate" storage class to be something that is
# like a dict but is not a dict. This will fail unless storage
# class conversion is supported in put and get.
Expand All @@ -207,7 +206,11 @@ def test_from_pipeline_intermediates_differ(self):
storageClass="TaskMetadataLike",
)
)

executor = self._configure_pipeline(
NoDimensionsTestTask.ConfigClass,
NoDimensionsTestTask.ConfigClass,
storageClass_b="TaskMetadataLike",
)
with self.assertLogs("lsst", level="INFO") as cm:
quanta = executor.run(register_dataset_types=True, save_versions=False)
# A dict is given to task a without change.
Expand All @@ -221,8 +224,8 @@ def test_from_pipeline_intermediates_differ(self):
self._test_logs(cm.output, "dict", "dict", "dict", "lsst.pipe.base.TaskMetadata")

self.assertEqual(len(quanta), 2)
self.assertEqual(self.butler.get("intermediate").to_dict(), {"zero": 0, "one": 1})
self.assertEqual(self.butler.get("output").to_dict(), {"zero": 0, "one": 1, "two": 2})
self.assertEqual(self.butler.get("intermediate"), TaskMetadata.from_dict({"zero": 0, "one": 1}))
self.assertEqual(self.butler.get("output"), TaskMetadata.from_dict({"zero": 0, "one": 1, "two": 2}))

def test_from_pipeline_output_differ(self):
"""Run pipeline but output definition in registry differs."""
Expand All @@ -236,13 +239,11 @@ def test_from_pipeline_output_differ(self):
storageClass="TaskMetadataLike",
)
)

executor = self._configure_pipeline(
NoDimensionsTestTask.ConfigClass,
NoDimensionsTestTask.ConfigClass,
storageClass_a="TaskMetadataLike",
)

with self.assertLogs("lsst", level="INFO") as cm:
quanta = executor.run(register_dataset_types=True, save_versions=False)
# a has been told to return a TaskMetadata but this will convert to
Expand All @@ -251,8 +252,8 @@ def test_from_pipeline_output_differ(self):
self._test_logs(cm.output, "dict", "lsst.pipe.base.TaskMetadata", "dict", "dict")

self.assertEqual(len(quanta), 2)
self.assertEqual(self.butler.get("intermediate").to_dict(), {"zero": 0, "one": 1})
self.assertEqual(self.butler.get("output").to_dict(), {"zero": 0, "one": 1, "two": 2})
self.assertEqual(self.butler.get("intermediate"), TaskMetadata.from_dict({"zero": 0, "one": 1}))
self.assertEqual(self.butler.get("output"), TaskMetadata.from_dict({"zero": 0, "one": 1, "two": 2}))

def test_from_pipeline_input_differ(self):
"""Run pipeline but input definition in registry differs."""
Expand All @@ -268,8 +269,11 @@ def test_from_pipeline_input_differ(self):
self.assertEqual(self.butler.get("intermediate"), {"zero": 0, "one": 1})
self.assertEqual(self.butler.get("output"), {"zero": 0, "one": 1, "two": 2})

def test_from_pipeline_incompatible(self):
"""Run pipeline but definitions are not compatible."""
def test_from_pipeline_inconsistent_dataset_types(self):
"""Generate the QG (by initializing the executor), then register the
dataset type with a different storage class than the QG should have
predicted, to make sure execution fails as it should.
"""
executor = self._configure_pipeline(
NoDimensionsTestTask.ConfigClass, NoDimensionsTestTask.ConfigClass
)
Expand Down

0 comments on commit 1a05997

Please sign in to comment.