From 7238d4b4bec13bb14f7976ef258d14e8834e94fb Mon Sep 17 00:00:00 2001 From: nstarman Date: Sat, 10 Oct 2026 19:23:22 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix:=20match=20the=20frozendict?= =?UTF-8?q?=20backport=20to=20the=20Python=203.15=20built-in?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A differential probe against CPython 3.15.0's frozendict found real behavioural gaps in the backport: - `d.__init__(...)` could replace the contents of an existing frozendict. Construction now happens in `__new__`, as for the built-in. - `frozendict(d) is d` for an exact frozendict, as for the built-in. - `|` merged any Mapping; the built-in merges only dict/frozendict. - `copy.copy` of an exact frozendict returns it; subclasses copy via pickle. - Pickling goes through `__getnewargs__`/`__newobj__`; protocols 0-1 are refused. - The hash error message and the too-many-arguments message match. The remaining differences are documented in the module docstring. Co-Authored-By: Claude Opus 5.5 --- src/xmmutablemap/_frozendict.py | 143 +++++++++++++++++++++++++++----- tests/test_immutablemap.py | 104 +++++++++++++++++++---- 2 files changed, 210 insertions(+), 37 deletions(-) diff --git a/src/xmmutablemap/_frozendict.py b/src/xmmutablemap/_frozendict.py index ea7e893..dd9fa5d 100644 --- a/src/xmmutablemap/_frozendict.py +++ b/src/xmmutablemap/_frozendict.py @@ -8,10 +8,23 @@ :class:`~xmmutablemap.ImmutableMap` can subclass ``frozendict`` on every supported version. -Behaviour is matched to the built-in wherever it is observable, including the -order-independent hash, equality with :class:`dict`, the ``frozendict({...})`` -repr (and bare ``frozendict()`` when empty), and ``|`` returning a plain -``frozendict`` rather than the subclass. +Behaviour is matched to the built-in (checked against CPython 3.15.0), +including construction in ``__new__``, ``frozendict(d) is d`` for an exact +``frozendict``, the hash (that of the items' ``frozenset``), equality with +:class:`dict`, the ``frozendict({...})`` repr (and bare ``frozendict()`` when +empty), ``|`` merging only a ``dict`` or ``frozendict`` and returning a plain +``frozendict`` rather than the subclass, ``copy.copy``, and pickling. + +Known differences, all either inherent to a pure-Python class or cosmetic: + +- It subclasses :class:`collections.abc.Mapping` (so static type checkers see + a ``Mapping``); the built-in is only registered as one. This shows in the + MRO, ``dir()``, and the ``ABCMeta`` metaclass. +- ``.keys().mapping`` (and the other views') wraps the backing ``dict``, not + the ``frozendict``. +- Some error messages name ``dict`` (e.g. for unhashable keys) where the + built-in names ``frozendict``; the exception types match. +- ``__module__``, ``inspect.signature``, and the like differ. """ __all__ = ("frozendict",) @@ -24,7 +37,9 @@ Mapping, ValuesView, ) -from typing import Any, TypeVar, cast, overload +from copyreg import __newobj__ # type: ignore[attr-defined] +from typing import Any, SupportsIndex, TypeVar, cast, overload +from typing_extensions import Self _T = TypeVar("_T") K = TypeVar("K") @@ -57,17 +72,30 @@ class frozendict(Mapping[K, V]): # noqa: N801 # pylint: disable=invalid-name # `__slots__` alone gives instances no `__dict__`, so attributes can be # neither set nor deleted -- with the same message the built-in raises. # No `__setattr__` override is needed to reproduce that. + # ponytail: `_data` itself stays rebindable, being a slot; it is private. __slots__ = ("_data",) _data: dict[K, V] - def __init__( - self, + # Built in `__new__`, not `__init__`, as the built-in is: calling + # `__init__` again must not change the contents, and a subclass's own + # `__init__` sees an already-filled mapping. + def __new__( + cls, /, *args: Mapping[K, V] | Iterable[tuple[K, V]], **kwargs: V, - ) -> None: + ) -> "Self": + if len(args) > 1: + msg = f"frozendict expected at most 1 argument, got {len(args)}" + raise TypeError(msg) + # Like `tuple(t)`, an exact frozendict of an exact frozendict is itself. + # pylint: disable-next=unidiomatic-typecheck + if cls is frozendict and args and not kwargs and type(args[0]) is frozendict: + return cast("Self", args[0]) + self = super().__new__(cls) self._data = dict(*args, **kwargs) # type: ignore[assignment] + return self # =========================================== # Mapping protocol @@ -190,7 +218,7 @@ def get(self, key: K, /, default: V | _T | None = None) -> V | _T | None: """ return self._data.get(key, default) - def copy(self) -> "frozendict[K, V]": + def copy(self, /) -> "frozendict[K, V]": """Return a shallow copy. As the built-in does, an exact `frozendict` returns itself; a subclass @@ -210,6 +238,67 @@ def copy(self) -> "frozendict[K, V]": # pylint: disable-next=unidiomatic-typecheck return self if type(self) is frozendict else frozendict(self._data) + def __copy__(self) -> "frozendict[K, V]": + """Support `copy.copy`. + + The `copy` module treats an exact built-in `frozendict` as atomic and + copies a subclass through its pickle protocol. Match both. + + Examples + -------- + >>> import copy + >>> from xmmutablemap import frozendict + >>> d = frozendict(a=1) + >>> copy.copy(d) is d + True + + """ + # pylint: disable-next=unidiomatic-typecheck + if type(self) is frozendict: + return self + func, args, state, *_ = self.__reduce_ex__(4) + new = func(*args) + if state: + new.__dict__.update(state) + return cast("frozendict[K, V]", new) + + def __getnewargs__(self) -> tuple[dict[K, V]]: + """Return the arguments `__new__` needs to rebuild this. + + Examples + -------- + >>> from xmmutablemap import frozendict + >>> frozendict(a=1).__getnewargs__() + ({'a': 1},) + + """ + return (dict(self._data),) + + def __reduce_ex__(self, protocol: SupportsIndex, /) -> tuple[Any, ...]: + """Reduce for pickling and `copy`, exactly as the built-in does. + + The data is the `__new__` argument, and only a subclass's instance + `__dict__` (if any) is state. Spelled out here, rather than left to + `object`, so pickles rebuild through `__new__`; protocols 0 and 1, + which cannot, are refused as the built-in refuses them. + + Examples + -------- + >>> from xmmutablemap import frozendict + >>> frozendict(a=1).__reduce_ex__(4)[1:] + ((, {'a': 1}), None, None, None) + + """ + if protocol < 2: # type: ignore[operator] + # The built-in, needing `__newobj__`, refuses protocols 0 and 1, + # naming `frozendict` even for a subclass. + msg = "cannot pickle 'frozendict' object" + raise TypeError(msg) + # ponytail: extra `__slots__` on a subclass are not pickled; add them + # to the state if a subclass ever needs them. + state = getattr(self, "__dict__", None) or None + return __newobj__, (type(self), *self.__getnewargs__()), state, None, None + @classmethod def fromkeys( cls, iterable: Iterable[K], value: Any = None, / @@ -238,7 +327,9 @@ def __or__(self, other: Any, /) -> "frozendict[Any, Any]": frozendict({'a': 1, 'b': 2}) """ - if not isinstance(other, Mapping): + # The built-in merges only a `dict` or `frozendict`; any other + # mapping must bring its own `|` (as `mappingproxy` does). + if not isinstance(other, (dict, frozendict)): return NotImplemented # The built-in's C implementation hardcodes the result type, so a # subclass of `frozendict` gets a plain `frozendict` back. Match that. @@ -254,11 +345,9 @@ def __ror__(self, other: Any, /) -> Any: The left operand drives the merge, which reproduces the built-in's results -- including keeping the left-hand type. - A `Mapping` that implements no ``|`` of its own raises `TypeError` - here, exactly as it does for the built-in: ``__ror__`` is not a - promise to merge into any mapping whatsoever. The one difference is - cosmetic -- the message names ``dict``, this class's backing store, - where the built-in names ``frozendict``. + As with the built-in, only a `dict` (or subclass) merges this way; + any other `Mapping` gets ``NotImplemented``, so ``|`` raises + `TypeError` unless that mapping brings its own ``|``. Examples -------- @@ -270,11 +359,11 @@ def __ror__(self, other: Any, /) -> Any: 'OrderedDict' """ - if not isinstance(other, Mapping): + if not isinstance(other, dict): return NotImplemented - # `other` is some Mapping; let it drive the merge so that e.g. - # `OrderedDict | frozendict` stays an OrderedDict. - return cast("Any", other) | self._data + # Let the dict drive the merge so that e.g. `OrderedDict | frozendict` + # stays an OrderedDict. + return other | self._data # =========================================== # Other @@ -312,18 +401,26 @@ def __eq__(self, other: object) -> bool: def __hash__(self) -> int: """Hash, order-independently, as the built-in does. - The hash *value* differs from the built-in's -- hashes are not stable - across interpreters or releases -- but the semantics match: equal - mappings hash equally whatever their insertion order. + The value is that of the items' `frozenset`, which is exactly the + built-in's hash. Examples -------- >>> from xmmutablemap import frozendict >>> hash(frozendict(a=1, b=2)) == hash(frozendict(b=2, a=1)) True + >>> hash(frozendict(a=1)) == hash(frozenset({("a", 1)})) + True """ - return hash(frozenset(self._data.items())) + try: + return hash(frozenset(self._data.items())) + except TypeError: + # Re-raise as the built-in does, naming the unhashable value + # rather than the `(key, value)` tuple holding it. + for value in self._data.values(): + hash(value) + raise # pragma: no cover # only if a value's hash is flaky def __repr__(self) -> str: """Return the representation. diff --git a/tests/test_immutablemap.py b/tests/test_immutablemap.py index cdff187..e2e6d60 100644 --- a/tests/test_immutablemap.py +++ b/tests/test_immutablemap.py @@ -1,5 +1,6 @@ """Test :mod:`xmmutablemap`.""" +import copy import pickle from collections import OrderedDict from collections.abc import Iterator, Mapping @@ -12,6 +13,22 @@ from xmmutablemap import ImmutableMap, frozendict +class Plain(Mapping[str, int]): + """A valid Mapping with no `__or__`.""" + + def __init__(self, d: dict[str, int]) -> None: # noqa: D107 + self._d = d + + def __getitem__(self, k: str) -> int: # noqa: D105 + return self._d[k] + + def __iter__(self) -> Iterator[str]: # noqa: D105 + return iter(self._d) + + def __len__(self) -> int: # noqa: D105 + return len(self._d) + + class TestImmutableMap: """Test :class:`ImmutableMap`.""" @@ -293,24 +310,83 @@ def test_ror_with_mapping_that_has_no_or(self) -> None: merge at all cannot merge here either. Verified against the 3.15 built-in, which raises the same way. """ + with pytest.raises(TypeError, match="'Plain' and 'frozendict'"): + _ = Plain({"b": 2}) | frozendict(a=1) - class Plain(Mapping[str, int]): - """A valid Mapping with no `__or__`.""" - - def __init__(self, d: dict[str, int]) -> None: - self._d = d - - def __getitem__(self, k: str) -> int: - return self._d[k] + def test_or_only_merges_dicts(self) -> None: + """`|` merges only a `dict` or `frozendict`, not any `Mapping`.""" + with pytest.raises(TypeError, match="'frozendict' and 'Plain'"): + _ = frozendict(a=1) | Plain({"b": 2}) + # A mappingproxy has its own `|`, which unwraps it to a dict. + assert frozendict(a=1) | MappingProxyType({"b": 2}) == {"a": 1, "b": 2} + assert type(OrderedDict(b=2) | frozendict(a=1)) is OrderedDict - def __iter__(self) -> Iterator[str]: - return iter(self._d) + def test_init_cannot_mutate(self) -> None: + """Construction happens in `__new__`; calling `__init__` is a no-op.""" + d = frozendict(a=1) + d.__init__(b=2) # type: ignore[misc] + assert d == {"a": 1} - def __len__(self) -> int: - return len(self._d) + def test_too_many_args(self) -> None: + """More than one positional argument names `frozendict`.""" + with pytest.raises(TypeError, match="^frozendict expected at most 1"): + frozendict({}, {}) # type: ignore[call-overload] - with pytest.raises(TypeError, match="unsupported operand type"): - _ = Plain({"b": 2}) | frozendict(a=1) + def test_constructor_returns_same_frozendict(self) -> None: + """`frozendict(d)` of an exact `frozendict` is `d`, as for `tuple`.""" + d = frozendict(a=1) + assert frozendict(d) is d + assert frozendict(d, b=2) is not d + assert type(frozendict(ImmutableMap(a=1))) is frozendict + + def test_subclass_init_receives_args(self) -> None: + """A subclass `__init__` runs after `__new__` has built the data.""" + + class Sub(frozendict[str, int]): + def __init__(self, *_args: Any, **_kwargs: Any) -> None: + self.seen = dict(self) + + assert Sub(a=1).seen == {"a": 1} + + def test_hash_is_frozenset_of_items(self) -> None: + """The hash is that of the items' frozenset, as for the built-in.""" + d = frozendict(a=1, b=2) + assert hash(d) == hash(frozenset(d.items())) + with pytest.raises(TypeError, match="^unhashable type: 'list'$"): + hash(frozendict(a=[1])) + + def test_copy_module(self) -> None: + """`copy.copy` returns an exact `frozendict` itself; deepcopy copies.""" + d = frozendict(a=[1]) + assert copy.copy(d) is d + deep = copy.deepcopy(d) + assert deep == d + assert deep["a"] is not d["a"] + + class Sub(frozendict[str, Any]): + pass + + s = Sub(a=1) + s.x = 3 # type: ignore[attr-defined] + c = copy.copy(s) + assert type(c) is Sub + assert c is not s + assert c == s + assert c.x == 3 # type: ignore[attr-defined] + + def test_getnewargs(self) -> None: + """Pickling goes through `__getnewargs__`, with a copy of the data.""" + d = frozendict(a=1) + (data,) = d.__getnewargs__() + assert data == {"a": 1} + data["b"] = 2 + assert d == {"a": 1} + + @pytest.mark.parametrize("protocol", [0, 1]) + def test_old_pickle_protocols_refused(self, protocol: int) -> None: + """Protocols 0 and 1 cannot rebuild through `__new__`, so they raise.""" + with pytest.raises(TypeError, match="cannot pickle 'frozendict' object"): + pickle.dumps(frozendict(a=1), protocol) @pytest.mark.parametrize("cls", [frozendict, ImmutableMap]) def test_no_attribute_mutation(self, cls: type) -> None: