diff --git a/doc/changes/DM-55542.bugfix.md b/doc/changes/DM-55542.bugfix.md new file mode 100644 index 0000000000..58220e5ef2 --- /dev/null +++ b/doc/changes/DM-55542.bugfix.md @@ -0,0 +1,2 @@ +Fixed `DimensionConfig.from_simple` so that the resulting configuration contains only JSON-compatible types. +Previously set-typed fields survived the Pydantic conversion, causing `Config.dump(format="json")` (and thus `Butler.makeRepo` with a `RemoteButler` dimension configuration) to fail with `TypeError`. diff --git a/doc/changes/DM-55542.feature.md b/doc/changes/DM-55542.feature.md new file mode 100644 index 0000000000..834d40cf09 --- /dev/null +++ b/doc/changes/DM-55542.feature.md @@ -0,0 +1,3 @@ +Added `DatasetType.conform_to` and `DatasetRef.conform_to` for rebuilding a dataset type or ref in a different but compatible dimension universe. +Conforming requires that the universes share a namespace and that the dimension group has the same names and the same required/implied split in both universes; otherwise `InconsistentUniverseError` is raised. +`Butler.transfer_from` now uses this API when importing refs from another universe. diff --git a/python/lsst/daf/butler/_dataset_ref.py b/python/lsst/daf/butler/_dataset_ref.py index a7a6c5a6ad..2a52603c0d 100644 --- a/python/lsst/daf/butler/_dataset_ref.py +++ b/python/lsst/daf/butler/_dataset_ref.py @@ -828,6 +828,44 @@ def replace( datastore_records=datastore_records, ) + def conform_to(self, universe: DimensionUniverse) -> DatasetRef: + """Rebuild this dataset ref in a different dimension universe. + + Parameters + ---------- + universe : `DimensionUniverse` + Target dimension universe. + + Returns + ------- + ref : `DatasetRef` + This ref if its dataset type's dimensions already belong to + ``universe``, else an otherwise-identical ref whose dataset type + and data ID belong to ``universe``. The data ID of a rebuilt ref + holds only the required dimension values; any implied values or + attached dimension records are dropped and must be recovered by + expanding the data ID against a data repository that uses the + target universe. + + Raises + ------ + InconsistentUniverseError + Raised if the dataset type cannot be rebuilt in the target + universe (see `DatasetType.conform_to`). + """ + if self.datasetType.dimensions.universe is universe: + return self + datasetType = self.datasetType.conform_to(universe) + dataId = DataCoordinate.standardize(dict(self.dataId.required), dimensions=datasetType.dimensions) + return DatasetRef( + datasetType=datasetType, + dataId=dataId, + run=self.run, + id=self.id, + conform=False, + datastore_records=self._datastore_records, + ) + def is_compatible_with(self, other: DatasetRef) -> bool: """Determine if the given `DatasetRef` is compatible with this one. diff --git a/python/lsst/daf/butler/_dataset_type.py b/python/lsst/daf/butler/_dataset_type.py index 4e812ed262..92b3f8f845 100644 --- a/python/lsst/daf/butler/_dataset_type.py +++ b/python/lsst/daf/butler/_dataset_type.py @@ -38,7 +38,7 @@ from pydantic import BaseModel, StrictBool, StrictStr from ._config_support import LookupKey -from ._exceptions import UnknownComponentError +from ._exceptions import InconsistentUniverseError, UnknownComponentError from ._storage_class import StorageClass, StorageClassFactory from .dimensions import DimensionGroup from .json import from_json_pydantic, to_json_pydantic @@ -343,6 +343,60 @@ class for this dataset type that can convert the python type associated return self_sc.can_convert(other_sc) + def conform_to(self, universe: DimensionUniverse) -> DatasetType: + """Rebuild this dataset type in a different dimension universe. + + Parameters + ---------- + universe : `DimensionUniverse` + Target dimension universe. + + Returns + ------- + dataset_type : `DatasetType` + This dataset type if its dimensions already belong to + ``universe``, else an otherwise-identical dataset type whose + dimensions belong to ``universe``. + + Raises + ------ + InconsistentUniverseError + Raised if the target universe has a different namespace, does not + contain all of this dataset type's dimensions, or conforms those + dimensions to a group with different names or a different split + between required and implied dimensions. + """ + source_universe = self._dimensions.universe + if source_universe is universe: + return self + if source_universe.namespace != universe.namespace: + raise InconsistentUniverseError( + f"Dataset type {self._name!r} has universe {source_universe} with a different " + f"namespace than target universe {universe}." + ) + try: + dimensions = universe.conform(self._dimensions.names) + except KeyError as exc: + raise InconsistentUniverseError( + f"Dimensions {self._dimensions} of dataset type {self._name!r} from universe " + f"{source_universe} do not all exist in target universe {universe}." + ) from exc + if dimensions.names != self._dimensions.names or set(dimensions.required) != set( + self._dimensions.required + ): + raise InconsistentUniverseError( + f"Dimensions {self._dimensions} of dataset type {self._name!r} from universe " + f"{source_universe} are different from the conforming set of target universe " + f"{universe} dimensions {dimensions}." + ) + return DatasetType( + self._name, + dimensions, + self._storageClass or self._storageClassName, + parentStorageClass=self._parentStorageClass or self._parentStorageClassName, + isCalibration=self._isCalibration, + ) + def __hash__(self) -> int: """Hash DatasetType instance. diff --git a/python/lsst/daf/butler/dimensions/_config.py b/python/lsst/daf/butler/dimensions/_config.py index 5afcf2d067..e0a431018d 100644 --- a/python/lsst/daf/butler/dimensions/_config.py +++ b/python/lsst/daf/butler/dimensions/_config.py @@ -183,11 +183,16 @@ def from_simple(simple: SerializedDimensionConfig) -> DimensionConfig: """ return DimensionConfig( simple.model_dump( + # Force sets back to lists for storage in the config. + # The config does not do this sanitation itself and so + # without this the config can not be serialized to JSON + # form using the dump() method. + mode="json", # Some of the fields in Pydantic model config have aliases # (e.g. remapping 'class_' to 'class'). Pydantic ignores these # in model_dump() by default, so we have to add by_alias to # make sure that we end up with the right names in the dict. - by_alias=True + by_alias=True, ) ) diff --git a/python/lsst/daf/butler/direct_butler/_direct_butler.py b/python/lsst/daf/butler/direct_butler/_direct_butler.py index ad0939fae5..ec0993753c 100644 --- a/python/lsst/daf/butler/direct_butler/_direct_butler.py +++ b/python/lsst/daf/butler/direct_butler/_direct_butler.py @@ -71,7 +71,6 @@ DatasetNotFoundError, DimensionValueError, EmptyQueryResultError, - InconsistentUniverseError, ValidationError, ) from .._file_dataset import FileDataset @@ -2182,42 +2181,7 @@ def _cast_universe_for_import_refs( refs_by_type: defaultdict[DatasetType, list[DatasetRef]] = defaultdict(list) for source_type, refs in refs_by_source_type.items(): - source_universe = source_type.dimensions.universe - if source_universe is self.dimensions: - target_type = source_type - else: - if source_universe.namespace != self.dimensions.namespace: - raise InconsistentUniverseError( - f"Source refs have universe {source_universe} with different namespace " - f"than target universe {self.dimensions}." - ) - - # Try to handle case of different universe versions. For now - # we can only do trivial check that dimension groups are - # identical. - try: - target_dimensions = self.dimensions.conform(source_type.dimensions.names) - except Exception as exc: - raise InconsistentUniverseError( - f"Source dimensions {source_type.dimensions} are not compatible with " - f"target universe dimensions {self.dimensions}." - ) from exc - if target_dimensions != source_type.dimensions: - raise InconsistentUniverseError( - f"Source dimensions {source_type.dimensions} are different from a conforming " - f"set of target universe dimensions {target_dimensions}." - ) - - # Rebuild dataset type in new universe. - target_type = DatasetType( - name=source_type.name, - dimensions=target_dimensions, - storageClass=source_type.storageClass, - parentStorageClass=source_type.parentStorageClass, - universe=self.dimensions, - isCalibration=source_type.isCalibration(), - ) - refs_by_type[target_type] = refs + refs_by_type[source_type.conform_to(self.dimensions)] = refs return refs_by_type diff --git a/python/lsst/daf/butler/tests/hybrid_butler_registry.py b/python/lsst/daf/butler/tests/hybrid_butler_registry.py index f43e93cb57..eabdac52fc 100644 --- a/python/lsst/daf/butler/tests/hybrid_butler_registry.py +++ b/python/lsst/daf/butler/tests/hybrid_butler_registry.py @@ -147,15 +147,11 @@ def getCollectionSummary(self, collection: str) -> CollectionSummary: def registerDatasetType(self, datasetType: DatasetType) -> bool: # We need to make sure that dataset type universe is the same as - # direct registry universe. + # direct registry universe. Only the remote universe is converted; + # anything else is passed through so that the direct registry can + # apply its strict universe check. if datasetType.dimensions.universe is self._remote.dimensions: - datasetType = DatasetType( - datasetType.name, - datasetType.dimensions.names, - datasetType.storageClass, - universe=self._direct.dimensions, - isCalibration=datasetType.isCalibration(), - ) + datasetType = datasetType.conform_to(self._direct.dimensions) return self._direct.registerDatasetType(datasetType) def removeDatasetType(self, name: str | tuple[str, ...]) -> None: diff --git a/tests/test_datasets.py b/tests/test_datasets.py index ce8cabab2c..fd13e28caa 100644 --- a/tests/test_datasets.py +++ b/tests/test_datasets.py @@ -39,6 +39,7 @@ DimensionConfig, DimensionUniverse, FileDataset, + InconsistentUniverseError, SerializedDatasetRefContainerV1, StorageClass, StorageClassFactory, @@ -1004,6 +1005,161 @@ def test_dataset_provenance(self) -> None: DatasetProvenance.strip_provenance_from_flat_dict(prov_dict) +class ConformToUniverseTestCase(unittest.TestCase): + """Tests for DatasetType.conform_to and DatasetRef.conform_to.""" + + def setUp(self) -> None: + self.universe = DimensionUniverse() + self.storageClass = StorageClass("test_conform_StructuredData") + + def _make_universe( + self, version_offset: int, **element_overrides: dict[str, object] | None + ) -> DimensionUniverse: + """Make a universe derived from the default one with a different + version and optional per-element configuration overrides. An + override of `None` removes the element entirely. + """ + config = DimensionConfig() + config["version"] = config["version"] + version_offset + for element, overrides in element_overrides.items(): + if overrides is None: + del config["elements", element] + else: + for key, value in overrides.items(): + config["elements", element, key] = value + return DimensionUniverse(config) + + def _load_old_universe(self, version: int) -> DimensionUniverse: + """Load a historical daf_butler universe from its frozen + configuration. + """ + config = DimensionConfig( + f"resource://lsst.daf.butler/configs/old_dimensions/daf_butler_universe{version}.yaml" + ) + return DimensionUniverse(config) + + def test_dataset_type_same_universe(self) -> None: + """Test that conforming to the dataset type's own universe returns + the same object. + """ + dataset_type = DatasetType( + "test", self.universe.conform(["detector"]), self.storageClass, isCalibration=True + ) + self.assertIs(dataset_type.conform_to(self.universe), dataset_type) + + def test_dataset_type_compatible_universe(self) -> None: + """Test conforming a dataset type to a different but compatible + universe. + """ + other = self._make_universe(1000) + dataset_type = DatasetType( + "test", self.universe.conform(["detector"]), self.storageClass, isCalibration=True + ) + conformed = dataset_type.conform_to(other) + self.assertIs(conformed.dimensions.universe, other) + self.assertEqual(conformed.dimensions.names, dataset_type.dimensions.names) + self.assertEqual(conformed.storageClass_name, dataset_type.storageClass_name) + self.assertTrue(conformed.isCalibration()) + self.assertEqual(conformed, dataset_type) + + def test_dataset_type_old_universe(self) -> None: + """Test conforming a dataset type between two real historical + universes in which the relevant dimension group is unchanged. + """ + # Both universes are pinned so that this test is unaffected by what + # the default universe happens to be. + universe7 = self._load_old_universe(7) + universe8 = self._load_old_universe(8) + dataset_type = DatasetType("test", universe7.conform(["visit"]), self.storageClass) + conformed = dataset_type.conform_to(universe8) + self.assertIs(conformed.dimensions.universe, universe8) + self.assertEqual(conformed.dimensions.names, dataset_type.dimensions.names) + # And back again. + round_tripped = conformed.conform_to(universe7) + self.assertIs(round_tripped.dimensions.universe, universe7) + self.assertEqual(round_tripped, dataset_type) + + def test_dataset_type_old_universe_incompatible(self) -> None: + """Test conforming a dataset type between two real historical + universes in which the relevant dimension group changed. + """ + # In universe 2 a visit group did not include day_obs, so it conforms + # to a larger group in universe 8. + universe2 = self._load_old_universe(2) + universe8 = self._load_old_universe(8) + dataset_type = DatasetType("test", universe2.conform(["visit"]), self.storageClass) + with self.assertRaisesRegex(InconsistentUniverseError, "different from the conforming set"): + dataset_type.conform_to(universe8) + + def test_dataset_type_component(self) -> None: + """Test conforming a component dataset type.""" + component_storage_class = StorageClass("test_conform_Component") + parent_storage_class = StorageClass( + "test_conform_Parent", + components={"a": component_storage_class, "b": component_storage_class}, + ) + other = self._make_universe(1000) + dataset_type = DatasetType( + "test.a", + self.universe.conform(["detector"]), + component_storage_class, + parentStorageClass=parent_storage_class, + ) + conformed = dataset_type.conform_to(other) + self.assertIs(conformed.dimensions.universe, other) + self.assertEqual(conformed, dataset_type) + + def test_dataset_type_missing_dimension(self) -> None: + """Test that conforming to a universe that lacks one of the dataset + type's dimensions fails. + """ + other = self._make_universe(1001, subfilter=None) + dataset_type = DatasetType("test", self.universe.conform(["subfilter"]), self.storageClass) + with self.assertRaisesRegex(InconsistentUniverseError, "do not all exist"): + dataset_type.conform_to(other) + + def test_dataset_type_changed_required(self) -> None: + """Test that conforming to a universe in which the same dimension + names have a different required/implied split fails. + """ + # In this universe a physical_filter group has the same dimension + # names as in the default universe, but band is required rather than + # implied. + other = self._make_universe(1002, physical_filter={"requires": ["instrument", "band"], "implies": []}) + dataset_type = DatasetType("test", self.universe.conform(["physical_filter"]), self.storageClass) + with self.assertRaisesRegex(InconsistentUniverseError, "different from the conforming set"): + dataset_type.conform_to(other) + + def test_dataset_type_different_namespace(self) -> None: + """Test that conforming to a universe with a different namespace + fails. + """ + config = DimensionConfig() + config["namespace"] = "test_conform" + other = DimensionUniverse(config) + dataset_type = DatasetType("test", self.universe.conform(["detector"]), self.storageClass) + with self.assertRaisesRegex(InconsistentUniverseError, "different namespace"): + dataset_type.conform_to(other) + + def test_dataset_ref(self) -> None: + """Test conforming a dataset ref to a different but compatible + universe. + """ + other = self._make_universe(1000) + dataset_type = DatasetType("test", self.universe.conform(["physical_filter"]), self.storageClass) + data_id = DataCoordinate.standardize( + {"instrument": "DummyCam", "physical_filter": "d-r", "band": "r"}, universe=self.universe + ) + self.assertTrue(data_id.hasFull()) + ref = DatasetRef(dataset_type, data_id, run="somerun") + self.assertIs(ref.conform_to(self.universe), ref) + conformed = ref.conform_to(other) + self.assertIs(conformed.datasetType.dimensions.universe, other) + self.assertEqual(conformed.id, ref.id) + self.assertEqual(conformed.run, ref.run) + self.assertEqual(dict(conformed.dataId.required), dict(ref.dataId.required)) + + class ZipIndexTestCase(unittest.TestCase): """Test that a ZipIndex can be read."""