From 84185b0936c7df0d173be7bdc6cc09efb6e728f4 Mon Sep 17 00:00:00 2001 From: Gabrieljr42 Date: Mon, 19 Oct 2026 16:03:57 -0300 Subject: [PATCH 1/4] Fix #636: implement sized items and values views for benedict Serializers such as toml-rs inspect len() on mapping.items(), which previously failed with a TypeError because benedict.items() and benedict.values() returned generators. Replace generator returns with _BenedictItemsView and _BenedictValuesView inheriting from collections.abc.ItemsView and ValuesView while delegating to BaseDict to preserve dynamic mutation reflection and nested dict casting. --- benedict/dicts/__init__.py | 40 ++++++++++++++++++---- tests/github/test_issue_0636.py | 59 +++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 tests/github/test_issue_0636.py diff --git a/benedict/dicts/__init__.py b/benedict/dicts/__init__.py index 9db46be1..8726d8c8 100644 --- a/benedict/dicts/__init__.py +++ b/benedict/dicts/__init__.py @@ -3,10 +3,12 @@ import re from collections.abc import ( Callable, + ItemsView, Iterable, Iterator, Mapping, Sequence, + ValuesView, ) from typing import ( Any, @@ -223,9 +225,8 @@ def invert(self, flat: bool = False) -> Self: """ return cast("Self", _invert(self, flat)) - def items(self) -> Iterator[tuple[_KPT, Any]]: # type: ignore[override] - for key, value in super().items(): - yield (key, self._cast(value)) # type: ignore[misc] + def items(self) -> ItemsView[_KPT, Any]: # type: ignore[override] + return _BenedictItemsView(self) def items_sorted_by_keys(self, reverse: bool = False) -> list[tuple[Any, Any]]: """ @@ -368,9 +369,36 @@ def unique(self) -> None: """ _unique(self) - def values(self) -> Iterator[Any]: # type: ignore[override] - for value in super().values(): - yield self._cast(value) + def values(self) -> ValuesView[Any]: # type: ignore[override] + return _BenedictValuesView(self) + + +class _BenedictItemsView(ItemsView[Any, Any]): + """ + Sized, dict-like items view that casts nested values on iteration. + """ + + _mapping: benedict[Any, Any] + + def __iter__(self) -> Iterator[tuple[Any, Any]]: + mapping = self._mapping + # use the parent implementation, which reads from the backing `_dict` + for key, value in super(benedict, mapping).items(): + yield key, mapping._cast(value) + + +class _BenedictValuesView(ValuesView[Any]): + """ + Sized, dict-like values view that casts nested values on iteration. + """ + + _mapping: benedict[Any, Any] + + def __iter__(self) -> Iterator[Any]: + mapping = self._mapping + # use the parent implementation, which reads from the backing `_dict` + for value in super(benedict, mapping).values(): + yield mapping._cast(value) # fix benedict json dumps support - #57 #59 #61 diff --git a/tests/github/test_issue_0636.py b/tests/github/test_issue_0636.py new file mode 100644 index 00000000..b5b8c18e --- /dev/null +++ b/tests/github/test_issue_0636.py @@ -0,0 +1,59 @@ +import unittest +from collections.abc import ItemsView, ValuesView + +from benedict import benedict + + +class github_issue_0636_test_case(unittest.TestCase): + """ + This class describes a github issue 0636 test case. + https://github.com/fabiocaccamo/python-benedict/issues/636 + + To run this specific test: + - Run python -m unittest tests.github.test_issue_0636 + """ + + def test_items_and_values_are_sized_views(self) -> None: + # serializers such as toml-rs call len() on mapping.items() + d = benedict({"foo": [{"bar": "baz"}], "nested": {"value": 1}}) + items = d.items() + values = d.values() + self.assertIsInstance(items, ItemsView) + self.assertIsInstance(values, ValuesView) + self.assertEqual(len(items), 2) + self.assertEqual(len(values), 2) + + def test_nested_items_are_sized_views(self) -> None: + d = benedict({"foo": [{"bar": "baz"}]}) + nested = d["foo"][0] + self.assertIsInstance(nested, benedict) + self.assertEqual(len(nested.items()), 1) + self.assertEqual(list(nested.items()), [("bar", "baz")]) + + def test_items_and_values_still_cast_nested_dicts(self) -> None: + source = {"nested": {"value": 1}} + d = benedict(source) + ((key, value),) = d.items() + self.assertEqual(key, "nested") + self.assertIsInstance(value, benedict) + self.assertIsInstance(list(d.values())[0], benedict) + # casting keeps the pointer to the original dict + value["value"] = 2 + self.assertEqual(source["nested"]["value"], 2) + + def test_items_and_values_reflect_mutations(self) -> None: + d = benedict({"a": 1}) + items = d.items() + values = d.values() + d["b"] = {"c": 2} + del d["a"] + self.assertEqual(list(d.keys()), ["b"]) + self.assertEqual(list(items), [("b", {"c": 2})]) + self.assertEqual(list(values), [{"c": 2}]) + self.assertEqual(len(items), 1) + + def test_items_membership(self) -> None: + d = benedict({"a": {"b": 1}}) + self.assertIn(("a", {"b": 1}), d.items()) + self.assertIn({"b": 1}, d.values()) + self.assertNotIn(("a", {"b": 2}), d.items()) From 06115039aa5fa7eae491ace01a6f6315a0a73578 Mon Sep 17 00:00:00 2001 From: Fabio Caccamo Date: Tue, 20 Oct 2026 13:28:09 +0200 Subject: [PATCH 2/4] Match actual keys in `items()` membership and support `reversed()` on views. --- benedict/dicts/__init__.py | 29 +++++++++++++++++++++++++++++ tests/github/test_issue_0636.py | 24 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/benedict/dicts/__init__.py b/benedict/dicts/__init__.py index 8726d8c8..b9836246 100644 --- a/benedict/dicts/__init__.py +++ b/benedict/dicts/__init__.py @@ -373,6 +373,14 @@ def values(self) -> ValuesView[Any]: # type: ignore[override] return _BenedictValuesView(self) +def _get_raw_value(mapping: benedict[Any, Any], key: Any) -> Any: + # read the value stored at the given key, without keypath/keylist parsing + raw = mapping.dict() + if raw is mapping: + return dict.__getitem__(mapping, key) + return raw[key] + + class _BenedictItemsView(ItemsView[Any, Any]): """ Sized, dict-like items view that casts nested values on iteration. @@ -380,12 +388,28 @@ class _BenedictItemsView(ItemsView[Any, Any]): _mapping: benedict[Any, Any] + def __contains__(self, item: object) -> bool: + # match actual keys only (like dict.items()), not keypaths + if not isinstance(item, tuple) or len(item) != 2: + return False + key, value = item + try: + current = _get_raw_value(self._mapping, key) + except (KeyError, TypeError): + return False + return current is value or current == value + def __iter__(self) -> Iterator[tuple[Any, Any]]: mapping = self._mapping # use the parent implementation, which reads from the backing `_dict` for key, value in super(benedict, mapping).items(): yield key, mapping._cast(value) + def __reversed__(self) -> Iterator[tuple[Any, Any]]: + mapping = self._mapping + for key in reversed(mapping.dict()): + yield key, mapping._cast(_get_raw_value(mapping, key)) + class _BenedictValuesView(ValuesView[Any]): """ @@ -400,6 +424,11 @@ def __iter__(self) -> Iterator[Any]: for value in super(benedict, mapping).values(): yield mapping._cast(value) + def __reversed__(self) -> Iterator[Any]: + mapping = self._mapping + for key in reversed(mapping.dict()): + yield mapping._cast(_get_raw_value(mapping, key)) + # fix benedict json dumps support - #57 #59 #61 JSONSerializer.disable_c_make_encoder() diff --git a/tests/github/test_issue_0636.py b/tests/github/test_issue_0636.py index b5b8c18e..f54b04f8 100644 --- a/tests/github/test_issue_0636.py +++ b/tests/github/test_issue_0636.py @@ -57,3 +57,27 @@ def test_items_membership(self) -> None: self.assertIn(("a", {"b": 1}), d.items()) self.assertIn({"b": 1}, d.values()) self.assertNotIn(("a", {"b": 2}), d.items()) + + def test_items_membership_does_not_resolve_keypaths(self) -> None: + # like dict.items(), membership only matches actual keys + for d in ( + benedict({"a": {"b": 1}, "l": [1, 2], "x": 1}), + benedict(a={"b": 1}, l=[1, 2], x=1), + ): + with self.subTest(pointer=d.dict() is not d): + self.assertIn(("x", 1), d.items()) + self.assertNotIn(("a.b", 1), d.items()) + self.assertNotIn(("l[0]", 1), d.items()) + self.assertNotIn((["a", "b"], 1), d.items()) + self.assertNotIn(("missing", 1), d.items()) + self.assertNotIn("x", d.items()) + + def test_items_and_values_are_reversible(self) -> None: + for d in ( + benedict({"a": 1, "b": {"c": 2}}), + benedict(a=1, b={"c": 2}), + ): + with self.subTest(pointer=d.dict() is not d): + self.assertEqual(list(reversed(d.items())), [("b", {"c": 2}), ("a", 1)]) + self.assertEqual(list(reversed(d.values())), [{"c": 2}, 1]) + self.assertIsInstance(next(reversed(d.values())), benedict) From 40e657e7f347ad4298a2cf8360c9f77964c9ae7f Mon Sep 17 00:00:00 2001 From: Fabio Caccamo Date: Tue, 20 Oct 2026 13:36:54 +0200 Subject: [PATCH 3/4] Simplify `items` / `values` views: reuse `BaseDict` views for iteration and membership. --- benedict/dicts/__init__.py | 52 +++++++++++++-------------------- tests/github/test_issue_0636.py | 15 +++++++++- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/benedict/dicts/__init__.py b/benedict/dicts/__init__.py index b9836246..4a12a570 100644 --- a/benedict/dicts/__init__.py +++ b/benedict/dicts/__init__.py @@ -3,6 +3,7 @@ import re from collections.abc import ( Callable, + Collection, ItemsView, Iterable, Iterator, @@ -43,6 +44,7 @@ from benedict.core import unflatten as _unflatten from benedict.core import unique as _unique from benedict.core.traverse import TraverseCallback +from benedict.dicts.base import BaseDict from benedict.dicts.io import IODict from benedict.dicts.keyattr import KeyattrDict from benedict.dicts.keylist import KeylistDict @@ -373,61 +375,49 @@ def values(self) -> ValuesView[Any]: # type: ignore[override] return _BenedictValuesView(self) -def _get_raw_value(mapping: benedict[Any, Any], key: Any) -> Any: - # read the value stored at the given key, without keypath/keylist parsing - raw = mapping.dict() - if raw is mapping: - return dict.__getitem__(mapping, key) - return raw[key] - - class _BenedictItemsView(ItemsView[Any, Any]): """ - Sized, dict-like items view that casts nested values on iteration. + Items view (like dict.items()) that casts nested values on iteration. + Membership uses the stored items, keys are not parsed as keypaths. """ _mapping: benedict[Any, Any] def __contains__(self, item: object) -> bool: - # match actual keys only (like dict.items()), not keypaths - if not isinstance(item, tuple) or len(item) != 2: - return False - key, value = item - try: - current = _get_raw_value(self._mapping, key) - except (KeyError, TypeError): - return False - return current is value or current == value + items: Collection[object] = BaseDict.items(self._mapping) + return item in items def __iter__(self) -> Iterator[tuple[Any, Any]]: - mapping = self._mapping - # use the parent implementation, which reads from the backing `_dict` - for key, value in super(benedict, mapping).items(): - yield key, mapping._cast(value) + cast_value = self._mapping._cast + for key, value in BaseDict.items(self._mapping): + yield key, cast_value(value) def __reversed__(self) -> Iterator[tuple[Any, Any]]: mapping = self._mapping - for key in reversed(mapping.dict()): - yield key, mapping._cast(_get_raw_value(mapping, key)) + for key in BaseDict.__reversed__(mapping): + yield key, mapping._cast(BaseDict.__getitem__(mapping, key)) class _BenedictValuesView(ValuesView[Any]): """ - Sized, dict-like values view that casts nested values on iteration. + Values view (like dict.values()) that casts nested values on iteration. """ _mapping: benedict[Any, Any] + def __contains__(self, value: object) -> bool: + # compare the stored values, without casting each of them + return value in BaseDict.values(self._mapping) + def __iter__(self) -> Iterator[Any]: - mapping = self._mapping - # use the parent implementation, which reads from the backing `_dict` - for value in super(benedict, mapping).values(): - yield mapping._cast(value) + cast_value = self._mapping._cast + for value in BaseDict.values(self._mapping): + yield cast_value(value) def __reversed__(self) -> Iterator[Any]: mapping = self._mapping - for key in reversed(mapping.dict()): - yield mapping._cast(_get_raw_value(mapping, key)) + for key in BaseDict.__reversed__(mapping): + yield mapping._cast(BaseDict.__getitem__(mapping, key)) # fix benedict json dumps support - #57 #59 #61 diff --git a/tests/github/test_issue_0636.py b/tests/github/test_issue_0636.py index f54b04f8..65a051bd 100644 --- a/tests/github/test_issue_0636.py +++ b/tests/github/test_issue_0636.py @@ -68,7 +68,9 @@ def test_items_membership_does_not_resolve_keypaths(self) -> None: self.assertIn(("x", 1), d.items()) self.assertNotIn(("a.b", 1), d.items()) self.assertNotIn(("l[0]", 1), d.items()) - self.assertNotIn((["a", "b"], 1), d.items()) + with self.assertRaises(TypeError): + # unhashable key (keylist), same as dict.items() + _ = (["a", "b"], 1) in d.items() self.assertNotIn(("missing", 1), d.items()) self.assertNotIn("x", d.items()) @@ -81,3 +83,14 @@ def test_items_and_values_are_reversible(self) -> None: self.assertEqual(list(reversed(d.items())), [("b", {"c": 2}), ("a", 1)]) self.assertEqual(list(reversed(d.values())), [{"c": 2}, 1]) self.assertIsInstance(next(reversed(d.values())), benedict) + + def test_values_membership(self) -> None: + for d in ( + benedict({"a": {"b": 1}, "x": 1}), + benedict(a={"b": 1}, x=1), + ): + with self.subTest(pointer=d.dict() is not d): + self.assertIn(1, d.values()) + self.assertIn({"b": 1}, d.values()) + self.assertIn(benedict({"b": 1}), d.values()) + self.assertNotIn({"b": 2}, d.values()) From ca4b4f34373545d09484d5679bd1d1ddedf659a1 Mon Sep 17 00:00:00 2001 From: Fabio Caccamo Date: Tue, 20 Oct 2026 13:37:34 +0200 Subject: [PATCH 4/4] Add regression tests for `items` / `values` views membership and stored keys. --- tests/github/test_issue_0636.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/github/test_issue_0636.py b/tests/github/test_issue_0636.py index 65a051bd..f40d231d 100644 --- a/tests/github/test_issue_0636.py +++ b/tests/github/test_issue_0636.py @@ -1,5 +1,6 @@ import unittest from collections.abc import ItemsView, ValuesView +from unittest.mock import patch from benedict import benedict @@ -94,3 +95,20 @@ def test_values_membership(self) -> None: self.assertIn({"b": 1}, d.values()) self.assertIn(benedict({"b": 1}), d.values()) self.assertNotIn({"b": 2}, d.values()) + + def test_membership_does_not_cast_values(self) -> None: + # membership compares the stored values, casting each one is not needed + d = benedict({f"k{i}": {"v": i} for i in range(10)}) + with patch.object(benedict, "_cast", side_effect=AssertionError("cast")): + self.assertIn({"v": 9}, d.values()) + self.assertIn(("k9", {"v": 9}), d.items()) + self.assertNotIn({"v": 10}, d.values()) + + def test_views_use_stored_keys_not_keypaths(self) -> None: + # a stored key like "x[0]" must not be parsed as a keypath with index + for d in (benedict({"x[0]": 1, "y": 2}), benedict(**{"x[0]": 1, "y": 2})): + with self.subTest(pointer=d.dict() is not d): + self.assertEqual(list(d.items()), [("x[0]", 1), ("y", 2)]) + self.assertEqual(list(reversed(d.items())), [("y", 2), ("x[0]", 1)]) + self.assertEqual(list(reversed(d.values())), [2, 1]) + self.assertIn(("x[0]", 1), d.items())