From f3eb476d80fa17f8f8eb80a36937b498449da768 Mon Sep 17 00:00:00 2001 From: "David H. Irving" Date: Fri, 1 Nov 2024 13:53:13 -0700 Subject: [PATCH 1/2] Fix dataset query missing implied dimensions Revert serialization change from DM-46776 in SerializedDataCoordinate, which caused it to drop values for implied dimensions. That change broke the transfer of DatasetRefs from server to client, since it drops the values for implied dimensions in the data ID. Per the documented behavior of dataset queries, the returned refs will have `hasFull() = True` and this behavior is relied on in tutorial notebooks. --- .../lsst/daf/butler/dimensions/_coordinate.py | 2 +- python/lsst/daf/butler/tests/butler_queries.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/python/lsst/daf/butler/dimensions/_coordinate.py b/python/lsst/daf/butler/dimensions/_coordinate.py index d511a49cac..d6f4189e9c 100644 --- a/python/lsst/daf/butler/dimensions/_coordinate.py +++ b/python/lsst/daf/butler/dimensions/_coordinate.py @@ -709,7 +709,7 @@ def to_simple(self, minimal: bool = False) -> SerializedDataCoordinate: else: records = None - return SerializedDataCoordinate(dataId=dict(self.required), records=records) + return SerializedDataCoordinate(dataId=dict(self.mapping), records=records) @classmethod def from_simple( diff --git a/python/lsst/daf/butler/tests/butler_queries.py b/python/lsst/daf/butler/tests/butler_queries.py index 0410afcef5..8360a86ed6 100644 --- a/python/lsst/daf/butler/tests/butler_queries.py +++ b/python/lsst/daf/butler/tests/butler_queries.py @@ -303,6 +303,24 @@ def test_simple_dataset_query(self) -> None: butler.query_datasets("bias", "*", detector=100, instrument="Unknown", find_first=False) self.assertIn("doomed", str(cm2.exception)) + # Test for a regression of an issue where "band" was not being included + # in the data ID, despite being one of the dimensions in the "flat" + # dataset type. + # + # "band" is implied by "physical_filter", so it's technically not a + # 'required' dimension. However, the contract of query_datasets is + # that hasFull() should be true, so implied dimensions must be + # included. + refs = butler.query_datasets("flat", "imported_r", where="detector = 2", instrument="Cam1") + self.assertEqual(len(refs), 1) + flat = refs[0] + self.assertTrue(flat.dataId.hasFull()) + self.assertEqual(flat.datasetType.name, "flat") + self.assertEqual(flat.dataId["instrument"], "Cam1") + self.assertEqual(flat.dataId["detector"], 2) + self.assertEqual(flat.dataId["physical_filter"], "Cam1-R1") + self.assertEqual(flat.dataId["band"], "r") + def test_general_query(self) -> None: """Test Query.general and its result.""" butler = self.make_butler("base.yaml", "datasets.yaml") From 7d49ac085632d86c4137755438003836855b4a84 Mon Sep 17 00:00:00 2001 From: "David H. Irving" Date: Fri, 1 Nov 2024 14:34:18 -0700 Subject: [PATCH 2/2] Add test for dataset type serialization --- tests/test_dimensions.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_dimensions.py b/tests/test_dimensions.py index 3a7fe2b9f6..a613572e33 100644 --- a/tests/test_dimensions.py +++ b/tests/test_dimensions.py @@ -42,6 +42,7 @@ DataCoordinate, DataCoordinateSequence, DataCoordinateSet, + DatasetType, Dimension, DimensionConfig, DimensionElement, @@ -411,6 +412,19 @@ def testPickling(self): group2 = pickle.loads(pickle.dumps(group1)) self.assertIs(group1, group2) + def testSerialization(self): + # Check that dataset types round-trip correctly through serialization. + dataset_type = DatasetType( + "flat", + dimensions=["instrument", "detector", "physical_filter", "band"], + isCalibration=True, + universe=self.universe, + storageClass="int", + ) + roundtripped = DatasetType.from_simple(dataset_type.to_simple(), universe=self.universe) + + self.assertEqual(dataset_type, roundtripped) + @dataclass class SplitByStateFlags: