Skip to content

Commit 592f1c1

Browse files
committed
Compute best_match relevance keys once, and settle sibling ties by position.
Follow-up to #1300, which finds the most relevant error within each anyOf / oneOf subschema before picking the deepest of those. This computes each context error's key exactly once rather than once per comparison, and breaks exact ties by preferring the error earliest in the instance, so that the result does not depend on the order in which errors were provided (which dropping `error.path` from `relevance` had otherwise reintroduced). Closes: #1257
1 parent a677cbe commit 592f1c1

3 files changed

Lines changed: 58 additions & 14 deletions

File tree

‎CHANGELOG.rst‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
v4.26.1
1+
v4.27.0
22
=======
33

4+
* Improve ``best_match`` for ``anyOf`` / ``oneOf`` errors: the most relevant error is now found within each subschema separately before the deepest of those is picked (#1257, #1300).
5+
In particular an applicator with a single subschema now produces the same best match as the bare subschema would.
6+
``relevance`` no longer considers an error's position amongst its siblings.
47
* Accessing an index with no errors on an ``ErrorTree`` no longer causes that index to appear in the tree (#1328).
58
Subtrees also properly propagate lookup errors which were previously missing when accessing indices not in the subtree.
69

‎jsonschema/exceptions.py‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from __future__ import annotations
55

66
from collections import deque
7+
from operator import itemgetter
78
from pprint import pformat
89
from textwrap import dedent, indent
910
from typing import TYPE_CHECKING, Any, ClassVar
@@ -497,25 +498,55 @@ def best_match(errors, key=relevance):
497498
set of inputs from version to version if better heuristics are added.
498499
499500
"""
500-
best = max(errors, key=key, default=None)
501-
if best is None:
501+
most_relevant = None
502+
for error in errors:
503+
most_relevant = _more_relevant(most_relevant, (key(error), error))
504+
if most_relevant is None:
502505
return
506+
_, best = most_relevant
503507

504508
while best.context:
505-
# Calculate the most relevant error in each separate subschema
506-
best_in_subschemas = []
509+
# Find the most relevant error within each separate subschema,
510+
# computing each error's key exactly once along the way.
511+
best_in_subschemas: dict[Any, tuple[Any, ValidationError]] = {}
507512
for error in best.context:
508-
index = error.schema_path[0]
509-
if index == len(best_in_subschemas):
510-
best_in_subschemas.append(error)
511-
else:
512-
prev = best_in_subschemas[index]
513-
best_in_subschemas[index] = max(prev, error, key=key)
513+
index = error.schema_path[0] if error.schema_path else None
514+
best_in_subschemas[index] = _more_relevant(
515+
best_in_subschemas.get(index),
516+
(key(error), error),
517+
)
514518

515519
# Calculate the minimum via nsmallest, because we don't recurse if
516520
# all nested errors have the same relevance (i.e. if min == max == all)
517-
smallest = heapq.nsmallest(2, best_in_subschemas, key=key)
518-
if len(smallest) == 2 and key(smallest[0]) == key(smallest[1]): # noqa: PLR2004
521+
smallest = heapq.nsmallest(
522+
2,
523+
best_in_subschemas.values(),
524+
key=itemgetter(0),
525+
)
526+
if len(smallest) == 2 and smallest[0][0] == smallest[1][0]: # noqa: PLR2004
519527
return best
520-
best = smallest[0]
528+
_, best = smallest[0]
521529
return best
530+
531+
532+
def _more_relevant(
533+
previous: tuple[Any, ValidationError] | None,
534+
candidate: tuple[Any, ValidationError],
535+
) -> tuple[Any, ValidationError]:
536+
"""
537+
Pick the more relevant of two ``(key, error)`` pairs.
538+
539+
Equally relevant errors are settled by picking the one which appears
540+
earlier in the instance, which makes the choice independent of the
541+
order in which the errors happened to be produced.
542+
"""
543+
if previous is None:
544+
return candidate
545+
previous_key, previous_error = previous
546+
candidate_key, candidate_error = candidate
547+
if candidate_key > previous_key or (
548+
candidate_key == previous_key
549+
and candidate_error.path < previous_error.path
550+
):
551+
return candidate
552+
return previous

‎jsonschema/tests/test_exceptions.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ def test_shallower_errors_are_better_matches(self):
3535
best = self.best_match_of(instance={"foo": {"bar": []}}, schema=schema)
3636
self.assertEqual(best.validator, "minProperties")
3737

38+
def test_earlier_sibling_errors_are_better_matches(self):
39+
"""
40+
Among equally relevant errors, the one earliest in the instance wins,
41+
regardless of the order in which the errors are provided.
42+
"""
43+
44+
schema = {"items": {"const": 37}}
45+
best = self.best_match_of(instance=[12, 12, 12], schema=schema)
46+
self.assertEqual(list(best.path), [0])
47+
3848
def test_oneOf_and_anyOf_are_weak_matches(self):
3949
"""
4050
A property you *must* match is probably better than one you have to

0 commit comments

Comments
 (0)