Repository navigation
fix: distinguish booleans from numbers in JSON Patch tests - #139
Conversation
|
Thanks @edsonsims. What do you think of this alternative implementation of
def _equal(left: object, right: object) -> bool: # noqa: PLR0911, PLR0912
"""Compare JSON values strictly without treating booleans as numbers."""
pending = [(left, right)]
seen: Set[Tuple[int, int]] = set()
while pending:
left, right = pending.pop()
if left is right:
continue
# bool can not be subclassed.
if type(left) is bool or type(right) is bool:
if left is not right:
return False
continue
# Remember strings are sequences too.
if isinstance(left, (str, bytes)):
if left == right:
continue
return False
pair = (id(left), id(right))
if pair in seen:
continue
if isinstance(left, Sequence):
if not (
issubclass(type(left), type(right))
or issubclass(type(right), type(left))
):
return False
if len(left) != len(right):
return False
seen.add(pair)
pending.extend(zip(left, right))
elif isinstance(left, Mapping):
if not (
issubclass(type(left), type(right))
or issubclass(type(right), type(left))
):
return False
if left.keys() != right.keys():
return False
seen.add(pair)
for key in left:
if type(key) is bool and type(right[key]) is not bool:
return False
pending.append((left[key], right[key]))
else:
if left == right:
continue
return False
return True |
|
Thanks, I adopted the Sequence/Mapping inheritance checks and earlier identity/seen handling in b21bd02. The mapping check compares the actual Boolean keys on both sides, rather than the value at right[key]. Matching {True: "foo"} maps still pass; True/1 and False/0 keys fail in both directions, including when their values are booleans. I made the str/bytes equality fallback symmetric, preserving UserString/str and bytearray/bytes comparisons in either direction. This is an explicit exception to the container inheritance rule. The early identity shortcut follows your proposal, including non-reflexive Python objects; these builder extensions are separate from RFC JSON semantics. Validation: all 73 added cases pass (37 fail against the previous PR head). The full suite passes 2,948 tests with one skip on each tested Python 3.10/3.12/3.13 environment. Ruff, formatting, strict Mypy, the official dual-configuration coverage workflow and installed-wheel checks pass. Iterative deep/cyclic container handling is retained. The PR description is unchanged. |
`test_json_patch_rfc6902.py` contains tests derived from the RFC and carries a copy of the IETF lisense.
We treat Mypy as the canonical type checker, but strive to please other common type chekers too.
|
👍 I've tweaked it a bit (see commit messages). In the future I plan to make JSON-like equality a configurable option. |
|
For the record, the performance hit from the current, iterative implementation of A recursive alternative that only handles An iterative version is somewhere in between. This kind of JSON-like deep equality is not limited to JSON Patch For now I'm inclined to lean towards correctness and safeness (non-recursive). |
JSON Patch
testcurrently uses Python equality, so it acceptstrueas equal to1, including inside arrays and objects. RFC 6902 section 4.6 requires the same JSON type recursively. I added a type-aware comparison for this operation while preserving numeric equality such as1and1.0, array order and object member-order independence. JSONPath filter comparisons are unchanged.The new cases expose 20 failures before the fix. All 45 added tests pass afterward, including unchanged controls, deep/shared containers, stopping a later operation, preserving atomic input and retaining equality semantics for Python string subclasses. Full suites pass 2,875 tests with one existing skip on Python 3.10, 3.12 and 3.13.
hatch run lint,hatch run typing,hatch run covand the package build pass; an installed-wheel probe outside the checkout also passes.