From a459a292c520d4515cae233e14f04ae4b68565d1 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 20 Feb 2024 13:42:00 -0500 Subject: [PATCH 1/7] Up-rev black in pre-commit. --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2c49c6885..61acbe996 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,7 +7,7 @@ repos: - id: trailing-whitespace - id: check-toml - repo: https://github.com/psf/black - rev: 23.12.1 + rev: 24.1.1 hooks: - id: black # It is recommended to specify the latest version of Python From ecb25bac3775d2ed4f069fe871717596bf94f41a Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 20 Feb 2024 13:32:27 -0500 Subject: [PATCH 2/7] Add methods to get and set nested dictionaries from TaskMetadata The methods here are trivial, but the PropertyList implementation is not, and it's the consistency between all three types (TaskMetadata, PropertyList, and PropertySet) that's really the value added here. --- doc/changes/DM-42928.feature.md | 3 + python/lsst/pipe/base/_task_metadata.py | 83 ++++++++++++++++++++++++- tests/test_taskmetadata.py | 20 ++++++ 3 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 doc/changes/DM-42928.feature.md diff --git a/doc/changes/DM-42928.feature.md b/doc/changes/DM-42928.feature.md new file mode 100644 index 000000000..618ad6cad --- /dev/null +++ b/doc/changes/DM-42928.feature.md @@ -0,0 +1,3 @@ +Add `TaskMetadata.get_dict` and `set_dict` methods. + +These provide a consistent way to assign and extract nested dictionaries from `TaskMetadata`, `lsst.daf.base.PropertySet`, and `lsst.daf.base.PropertyList`. diff --git a/python/lsst/pipe/base/_task_metadata.py b/python/lsst/pipe/base/_task_metadata.py index 79501a0ab..164e529de 100644 --- a/python/lsst/pipe/base/_task_metadata.py +++ b/python/lsst/pipe/base/_task_metadata.py @@ -25,13 +25,13 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -__all__ = ["TaskMetadata"] +__all__ = ["TaskMetadata", "SetDictMetadata", "GetDictMetadata", "NestedMetadataDict"] import itertools import numbers import sys from collections.abc import Collection, Iterator, Mapping, Sequence -from typing import Any, Protocol +from typing import Any, Protocol, TypeAlias, Union from pydantic import BaseModel, Field, StrictBool, StrictFloat, StrictInt, StrictStr @@ -39,6 +39,10 @@ # to allow predictable serialization. _ALLOWED_PRIMITIVE_TYPES = (str, float, int, bool) +# Note that '|' syntax for unions doesn't work when we have to use a string +# literal (and we do since it's recursive and not an annotation). +NestedMetadataDict: TypeAlias = Mapping[str, Union[str, float, int, bool, "NestedMetadataDict"]] + class PropertySetLike(Protocol): """Protocol that looks like a ``lsst.daf.base.PropertySet``. @@ -56,6 +60,41 @@ def _isListLike(v: Any) -> bool: return isinstance(v, Sequence) and not isinstance(v, str) +class SetDictMetadata(Protocol): + """Protocol for objects that can be assigned a possibly-nested `dict` of + primitives. + + This protocol is satisfied by `TaskMetadata`, `lsst.daf.base.PropertySet`, + and `lsst.daf.base.PropertyList`, providing a consistent way to insert a + dictionary into these objects that avoids their historical idiosyncrasies. + + The form in which these entries appear in the object's native keys and + values is implementation-defined. *Empty nested dictionaries *may* be + dropped, and if the top-level dictionary is empty this method may do + nothing.* + """ + + def set_dict(self, key: str, nested: NestedMetadataDict) -> None: ... + + +class GetDictMetadata(Protocol): + """Protocol for objects that can extract a possibly-nested mapping of + primitives. + + This protocol is satisfied by `TaskMetadata`, `lsst.daf.base.PropertySet`, + and `lsst.daf.base.PropertyList`, providing a consistent way to extract a + dictionary from these objects that avoids their historical idiosyncrasies. + + This is guaranteed to work for mappings inserted by + `~SetMapping.set_dict`. It should not be expected to work for values + inserted in other ways. If a value was never inserted with the given key + at all, *an empty `dict` will be returned* (this is a concession to + implementation constraints in `~lsst.daf.base.PropertyList`. + """ + + def get_dict(self, key: str) -> NestedMetadataDict: ... + + class TaskMetadata(BaseModel): """Dict-like object for storing task metadata. @@ -477,6 +516,46 @@ def __delitem__(self, key: str) -> None: # Report the correct key. raise KeyError(f"'{key}' not found'") from None + def get_dict(self, key: str) -> NestedMetadataDict: + """Return a possibly-hierarchical nested `dict`. + + This implements the `GetDictMetadata` protocol for consistency with + `lsst.daf.base.PropertySet` and `lsst.daf.base.PropertyList`. + + Parameters + ---------- + key : `str` + String key associated with the mapping. + + Returns + ------- + value : `~collections.abc.Mapping` + Possibly-nested mapping, with `str` keys and values that are `int`, + `float`, `str`, `bool`, or another `dict` with the same key and + value types. Will be empty if ``key`` does not exist. + """ + if value := self.get(key): + return value.to_dict() + else: + return {} + + def set_dict(self, key: str, value: NestedMetadataDict) -> None: + """Assign a possibly-hierarchical nested `dict`. + + This implements the `SetDictMetadata` protocol for consistency with + `lsst.daf.base.PropertySet` and `lsst.daf.base.PropertyList`. + + Parameters + ---------- + key : `str` + String key associated with the mapping. + value : `~collections.abc.Mapping` + Possibly-nested mapping, with `str` keys and values that are `int`, + `float`, `str`, `bool`, or another `dict` with the same key and + value types. + """ + self[key] = TaskMetadata.from_dict(value) + def _validate_value(self, value: Any) -> tuple[str, Any]: """Validate the given value. diff --git a/tests/test_taskmetadata.py b/tests/test_taskmetadata.py index 2cf918c81..cb24996fa 100644 --- a/tests/test_taskmetadata.py +++ b/tests/test_taskmetadata.py @@ -241,6 +241,26 @@ def testNumpy(self): with self.assertRaises(ValueError): meta["numpy"] = numpy.zeros(5) + def test_get_set_dict(self): + """Test the get_dict and set_dict methods.""" + obj = TaskMetadata() + d1 = {"one": 1, "two": 2.0, "three": True, "four": {"a": 4, "b": "B"}, "five": {}} + obj.set_dict("d", d1) + obj.set_dict("e", {}) + d2 = obj.get_dict("d") + # Keys with empty-dict values may or may not be round-tripped. + self.assertGreaterEqual(d2.keys(), {"one", "two", "three", "four"}) + self.assertLessEqual(d2.keys(), {"one", "two", "three", "four", "five"}) + self.assertEqual(d2["one"], d1["one"]) + self.assertEqual(d2["two"], d1["two"]) + self.assertEqual(d2["three"], d1["three"]) + self.assertEqual(d2["four"], d1["four"]) + self.assertEqual(d2.get("five", {}), d1["five"]) + # Empty dict may or may not have been added, and retrieving it or + # a key that was never added yields an empty dict. + self.assertEqual(obj.get_dict("e"), {}) + self.assertEqual(obj.get_dict("f"), {}) + if __name__ == "__main__": unittest.main() From 4c2d64cad4bf5a685abd891de0f119b80ed89a65 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 20 Feb 2024 14:49:40 -0500 Subject: [PATCH 3/7] fixup! Add methods to get and set nested dictionaries from TaskMetadata --- python/lsst/pipe/base/_task_metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lsst/pipe/base/_task_metadata.py b/python/lsst/pipe/base/_task_metadata.py index 164e529de..1a833116f 100644 --- a/python/lsst/pipe/base/_task_metadata.py +++ b/python/lsst/pipe/base/_task_metadata.py @@ -69,7 +69,7 @@ class SetDictMetadata(Protocol): dictionary into these objects that avoids their historical idiosyncrasies. The form in which these entries appear in the object's native keys and - values is implementation-defined. *Empty nested dictionaries *may* be + values is implementation-defined. *Empty nested dictionaries may be dropped, and if the top-level dictionary is empty this method may do nothing.* """ From 905cf02fbbd36c4970f64ca6e10eeb7b50b05713 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 20 Feb 2024 14:50:11 -0500 Subject: [PATCH 4/7] fixup! Add methods to get and set nested dictionaries from TaskMetadata --- python/lsst/pipe/base/_task_metadata.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/python/lsst/pipe/base/_task_metadata.py b/python/lsst/pipe/base/_task_metadata.py index 1a833116f..e203797a5 100644 --- a/python/lsst/pipe/base/_task_metadata.py +++ b/python/lsst/pipe/base/_task_metadata.py @@ -25,7 +25,13 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -__all__ = ["TaskMetadata", "SetDictMetadata", "GetDictMetadata", "NestedMetadataDict"] +__all__ = [ + "TaskMetadata", + "SetDictMetadata", + "GetDictMetadata", + "GetSetDictMetadata", + "NestedMetadataDict", +] import itertools import numbers @@ -95,6 +101,12 @@ class GetDictMetadata(Protocol): def get_dict(self, key: str) -> NestedMetadataDict: ... +class GetSetDictMetadata(SetDictMetadata, GetDictMetadata, Protocol): + """Protocol for objects that can assign and extract a possibly-nested + mapping of primitives. + """ + + class TaskMetadata(BaseModel): """Dict-like object for storing task metadata. @@ -520,7 +532,8 @@ def get_dict(self, key: str) -> NestedMetadataDict: """Return a possibly-hierarchical nested `dict`. This implements the `GetDictMetadata` protocol for consistency with - `lsst.daf.base.PropertySet` and `lsst.daf.base.PropertyList`. + `lsst.daf.base.PropertySet` and `lsst.daf.base.PropertyList`. The + returned `dict` is guaranteed to be a deep copy, not a view. Parameters ---------- @@ -554,7 +567,7 @@ def set_dict(self, key: str, value: NestedMetadataDict) -> None: `float`, `str`, `bool`, or another `dict` with the same key and value types. """ - self[key] = TaskMetadata.from_dict(value) + self[key] = value def _validate_value(self, value: Any) -> tuple[str, Any]: """Validate the given value. From dc0a9f2bab4b4838af21583ae3c01c8cad6e6d1d Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 20 Feb 2024 15:50:16 -0500 Subject: [PATCH 5/7] Prohibit '.' in TaskMetadata.get_dict/set_dict. --- python/lsst/pipe/base/_task_metadata.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/lsst/pipe/base/_task_metadata.py b/python/lsst/pipe/base/_task_metadata.py index e203797a5..200db90f9 100644 --- a/python/lsst/pipe/base/_task_metadata.py +++ b/python/lsst/pipe/base/_task_metadata.py @@ -78,6 +78,9 @@ class SetDictMetadata(Protocol): values is implementation-defined. *Empty nested dictionaries may be dropped, and if the top-level dictionary is empty this method may do nothing.* + + Neither the top-level key nor nested keys may contain ``.`` (period) + characters. """ def set_dict(self, key: str, nested: NestedMetadataDict) -> None: ... From 743adab1594194b0a24716f5d99dfc6c637cc793 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 20 Feb 2024 15:51:47 -0500 Subject: [PATCH 6/7] Add protocol for objects with a 'metadata' attribute. This will be used shortly on DM-39842. --- python/lsst/pipe/base/_status.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/python/lsst/pipe/base/_status.py b/python/lsst/pipe/base/_status.py index 11a41cfb5..caab95564 100644 --- a/python/lsst/pipe/base/_status.py +++ b/python/lsst/pipe/base/_status.py @@ -33,6 +33,18 @@ "InvalidQuantumError", ) +from typing import Protocol + +from ._task_metadata import GetSetDictMetadata + + +class GetSetDictMetadataHolder(Protocol): + """Protocol for objects that have a ``metadata`` attribute that satisfies + `GetSetDictMetadata`. + """ + + metadata: GetSetDictMetadata | None + class NoWorkFound(BaseException): """An exception raised when a Quantum should not exist because there is no From 9959a86314c33a37a1b303c438f0dc0e6be6bf2a Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 20 Feb 2024 15:59:04 -0500 Subject: [PATCH 7/7] fixup! Prohibit '.' in TaskMetadata.get_dict/set_dict. --- python/lsst/pipe/base/_task_metadata.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/python/lsst/pipe/base/_task_metadata.py b/python/lsst/pipe/base/_task_metadata.py index 200db90f9..07b0903a4 100644 --- a/python/lsst/pipe/base/_task_metadata.py +++ b/python/lsst/pipe/base/_task_metadata.py @@ -541,7 +541,8 @@ def get_dict(self, key: str) -> NestedMetadataDict: Parameters ---------- key : `str` - String key associated with the mapping. + String key associated with the mapping. May not have a ``.`` + character. Returns ------- @@ -564,11 +565,12 @@ def set_dict(self, key: str, value: NestedMetadataDict) -> None: Parameters ---------- key : `str` - String key associated with the mapping. + String key associated with the mapping. May not have a ``.`` + character. value : `~collections.abc.Mapping` Possibly-nested mapping, with `str` keys and values that are `int`, `float`, `str`, `bool`, or another `dict` with the same key and - value types. + value types. Nested keys may not have a ``.`` character. """ self[key] = value