From 4d005f7fecaae1d911da4e74f4d10568acd06a50 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 21 Nov 2023 13:54:07 -0500 Subject: [PATCH] Add, use DataCoordinate.make_empty. The old DataCoordinate.makeEmpty is not being deprecated because that wasn't included on RFC-834 and we don't have a good reason to get rid it, but we've now got a consistent suite of DataCoordinate factory methods with from_full_values and from_required_values (whose camelCase forms _did_ have a good reason to be retired, as this aided with the transition from DimensionGraph to DimensionGroup). --- .../lsst/daf/butler/dimensions/_coordinate.py | 32 +++++++++++++++---- .../daf/butler/registry/queries/_structs.py | 4 +-- .../queries/expressions/_predicate.py | 4 +-- .../daf/butler/registry/tests/_registry.py | 6 ++-- tests/test_butler.py | 2 +- tests/test_datastore.py | 4 +-- tests/test_dimensions.py | 4 +-- tests/test_expressions.py | 4 +-- tests/test_formatter.py | 2 +- 9 files changed, 41 insertions(+), 21 deletions(-) diff --git a/python/lsst/daf/butler/dimensions/_coordinate.py b/python/lsst/daf/butler/dimensions/_coordinate.py index f45a0bebf7..664ea08959 100644 --- a/python/lsst/daf/butler/dimensions/_coordinate.py +++ b/python/lsst/daf/butler/dimensions/_coordinate.py @@ -132,7 +132,7 @@ class DataCoordinate(NamedKeyMapping[Dimension, DataIdValue]): `DataCoordinate` is an ABC, but it provides `staticmethod` factory functions for private concrete implementations that should be sufficient for most purposes. `standardize` is the most flexible and safe of these; - the others (`makeEmpty`, `from_required_values`, and `from_full_values`) + the others (`make_empty`, `from_required_values`, and `from_full_values`) are more specialized and perform little or no checking of inputs. Lookups for implied dimensions (those in ``self.dimensions.implied``) are @@ -260,7 +260,7 @@ def standardize( raise TypeError("universe must be provided if graph is not.") dimensions = DimensionGroup(universe, new_mapping.keys()) if not dimensions: - return DataCoordinate.makeEmpty(universe) + return DataCoordinate.make_empty(universe) # Some backends cannot handle numpy.int64 type which is a subclass of # numbers.Integral; convert that to int. for k, v in new_mapping.items(): @@ -336,6 +336,26 @@ def makeEmpty(universe: DimensionUniverse) -> DataCoordinate: `hasRecords` are guaranteed to return `True`, because both `full` and `records` are just empty mappings. """ + return DataCoordinate.make_empty(universe) + + @staticmethod + def make_empty(universe: DimensionUniverse) -> DataCoordinate: + """Return an empty `DataCoordinate`. + + It identifies the null set of dimensions. + + Parameters + ---------- + universe : `DimensionUniverse` + Universe to which this null dimension set belongs. + + Returns + ------- + data_id : `DataCoordinate` + A data ID object that identifies no dimensions. `hasFull` and + `hasRecords` are guaranteed to return `True`, because both `full` + and `records` are just empty mappings. + """ return _ExpandedTupleDataCoordinate(universe.empty.as_group(), (), {}) # TODO: remove on DM-41326. @@ -390,7 +410,7 @@ def from_required_values(dimensions: DimensionGroup, values: tuple[DataIdValue, Returns ------- - dataId : `DataCoordinate` + data_id : `DataCoordinate` A data ID object that identifies the given dimensions. ``dataId.hasFull()`` will return `True` only if ``dimensions.implied`` is empty. ``dataId.hasRecords()`` will @@ -400,7 +420,7 @@ def from_required_values(dimensions: DimensionGroup, values: tuple[DataIdValue, values ), f"Inconsistency between dimensions {dimensions.required} and required values {values}." if not dimensions: - return DataCoordinate.makeEmpty(dimensions.universe) + return DataCoordinate.make_empty(dimensions.universe) if not dimensions.implied: return _FullTupleDataCoordinate(dimensions, values) return _RequiredTupleDataCoordinate(dimensions, values) @@ -461,7 +481,7 @@ def from_full_values(dimensions: DimensionGroup, values: tuple[DataIdValue, ...] Returns ------- - dataId : `DataCoordinate` + data_id : `DataCoordinate` A data ID object that identifies the given dimensions. ``dataId.hasFull()`` will always return `True`. ``dataId.hasRecords()`` will only return `True` if ``dimensions`` @@ -471,7 +491,7 @@ def from_full_values(dimensions: DimensionGroup, values: tuple[DataIdValue, ...] values ), f"Inconsistency between dimensions {dimensions.data_coordinate_keys} and full values {values}." if not dimensions: - return DataCoordinate.makeEmpty(dimensions.universe) + return DataCoordinate.make_empty(dimensions.universe) return _FullTupleDataCoordinate(dimensions, values) def __bool__(self) -> bool: diff --git a/python/lsst/daf/butler/registry/queries/_structs.py b/python/lsst/daf/butler/registry/queries/_structs.py index 6d6039536c..e3e1de9c32 100644 --- a/python/lsst/daf/butler/registry/queries/_structs.py +++ b/python/lsst/daf/butler/registry/queries/_structs.py @@ -108,9 +108,9 @@ def combine( An object representing the WHERE clause for a query. """ if data_id is None: - data_id = DataCoordinate.makeEmpty(dimensions.universe) + data_id = DataCoordinate.make_empty(dimensions.universe) if defaults is None: - defaults = DataCoordinate.makeEmpty(dimensions.universe) + defaults = DataCoordinate.make_empty(dimensions.universe) expression_predicate, governor_constraints = make_string_expression_predicate( expression, dimensions, diff --git a/python/lsst/daf/butler/registry/queries/expressions/_predicate.py b/python/lsst/daf/butler/registry/queries/expressions/_predicate.py index 637fc4316d..32d1b8160d 100644 --- a/python/lsst/daf/butler/registry/queries/expressions/_predicate.py +++ b/python/lsst/daf/butler/registry/queries/expressions/_predicate.py @@ -126,7 +126,7 @@ def make_string_expression_predicate( """ governor_constraints: dict[str, Set[str]] = {} if data_id is None: - data_id = DataCoordinate.makeEmpty(dimensions.universe) + data_id = DataCoordinate.make_empty(dimensions.universe) if not string: for dimension in data_id.dimensions.governors: governor_constraints[dimension] = {cast(str, data_id[dimension])} @@ -146,7 +146,7 @@ def make_string_expression_predicate( if column and table in dimensions.universe.elements.names: raise RuntimeError(f"Bind parameter key {identifier!r} looks like a dimension column.") if defaults is None: - defaults = DataCoordinate.makeEmpty(dimensions.universe) + defaults = DataCoordinate.make_empty(dimensions.universe) # Convert the expression to disjunctive normal form (ORs of ANDs). # That's potentially super expensive in the general case (where there's # a ton of nesting of ANDs and ORs). That won't be the case for the diff --git a/python/lsst/daf/butler/registry/tests/_registry.py b/python/lsst/daf/butler/registry/tests/_registry.py index ba8012f20b..fbe074c872 100644 --- a/python/lsst/daf/butler/registry/tests/_registry.py +++ b/python/lsst/daf/butler/registry/tests/_registry.py @@ -1685,7 +1685,7 @@ def testEmptyDimensionsQueries(self): self.loadData(registry, "base.yaml") schema = DatasetType("schema", dimensions=registry.dimensions.empty, storageClass="Catalog") registry.registerDatasetType(schema) - dataId = DataCoordinate.makeEmpty(registry.dimensions) + dataId = DataCoordinate.make_empty(registry.dimensions) run1 = "run1" run2 = "run2" registry.registerRun(run1) @@ -3339,8 +3339,8 @@ def test_long_query_names(self) -> None: registry.registerRun(run1) run2 = "run2" registry.registerRun(run2) - (ref1,) = registry.insertDatasets(name, [DataCoordinate.makeEmpty(registry.dimensions)], run1) - registry.insertDatasets(name, [DataCoordinate.makeEmpty(registry.dimensions)], run2) + (ref1,) = registry.insertDatasets(name, [DataCoordinate.make_empty(registry.dimensions)], run1) + registry.insertDatasets(name, [DataCoordinate.make_empty(registry.dimensions)], run2) self.assertEqual( set(registry.queryDatasets(name, collections=[run1, run2], findFirst=True)), {ref1}, diff --git a/tests/test_butler.py b/tests/test_butler.py index 70a0da4559..fcf9f85fca 100644 --- a/tests/test_butler.py +++ b/tests/test_butler.py @@ -2138,7 +2138,7 @@ def _absolute_transfer(self, transfer: str) -> None: metrics = makeExampleMetrics() with ResourcePath.temporary_uri(suffix=".json") as temp: - dataId = DataCoordinate.makeEmpty(self.source_butler.dimensions) + dataId = DataCoordinate.make_empty(self.source_butler.dimensions) source_refs = [DatasetRef(datasetType, dataId, run=run)] temp.write(json.dumps(metrics.exportAsDict()).encode()) dataset = FileDataset(path=temp, refs=source_refs) diff --git a/tests/test_datastore.py b/tests/test_datastore.py index 6c1bd7b1a3..d32c028781 100644 --- a/tests/test_datastore.py +++ b/tests/test_datastore.py @@ -945,7 +945,7 @@ def test_pydantic_dict_storage_class_conversions(self) -> None: "store_as_model", dimensions=self.universe.empty, storageClass="DictConvertibleModel", - dataId=DataCoordinate.makeEmpty(self.universe), + dataId=DataCoordinate.make_empty(self.universe), ) content = {"a": "one", "b": "two"} model = DictConvertibleModel.from_dict(content, extra="original content") @@ -988,7 +988,7 @@ def _assert_different_puts(self, datastore: Datastore, storageClass_root: str, d f"stora_as_{x}", dimensions=self.universe.empty, storageClass=f"{storageClass_root}{x}", - dataId=DataCoordinate.makeEmpty(self.universe), + dataId=DataCoordinate.make_empty(self.universe), ) for x in ["A", "B"] } diff --git a/tests/test_dimensions.py b/tests/test_dimensions.py index 4cd61de192..c43e8e7ea9 100644 --- a/tests/test_dimensions.py +++ b/tests/test_dimensions.py @@ -713,10 +713,10 @@ def testStandardize(self): DataCoordinate.standardize(dataId.mapping, universe=dataId.universe), DataCoordinate.standardize(dataId.mapping, dimensions=dataId.dimensions), DataCoordinate.standardize( - DataCoordinate.makeEmpty(dataId.dimensions.universe), **dataId.mapping + DataCoordinate.make_empty(dataId.dimensions.universe), **dataId.mapping ), DataCoordinate.standardize( - DataCoordinate.makeEmpty(dataId.dimensions.universe), + DataCoordinate.make_empty(dataId.dimensions.universe), dimensions=dataId.dimensions, **dataId.mapping, ), diff --git a/tests/test_expressions.py b/tests/test_expressions.py index 4b5334265f..59791ff371 100644 --- a/tests/test_expressions.py +++ b/tests/test_expressions.py @@ -355,8 +355,8 @@ def test_governor(self): universe = DimensionUniverse() dimensions = universe.conform(("instrument", "visit")) - dataId = DataCoordinate.makeEmpty(universe) - defaults = DataCoordinate.makeEmpty(universe) + dataId = DataCoordinate.make_empty(universe) + defaults = DataCoordinate.make_empty(universe) # governor-only constraint tree = parser.parse("instrument = 'LSST'") diff --git a/tests/test_formatter.py b/tests/test_formatter.py index ba5d6d21df..b59c25a72c 100644 --- a/tests/test_formatter.py +++ b/tests/test_formatter.py @@ -60,7 +60,7 @@ def setUp(self): self.id = 0 self.factory = FormatterFactory() self.universe = DimensionUniverse() - self.dataId = DataCoordinate.makeEmpty(self.universe) + self.dataId = DataCoordinate.make_empty(self.universe) # Dummy FileDescriptor for testing getFormatter self.fileDescriptor = FileDescriptor(