Improve AssertionError error message for sequences inside dictionaries
#12717
Conversation
psf-chronographer
bot
added
the
bot:chronographer:provided
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
| @@ -354,7 +354,7 @@ def _compare_eq_iterable( | |||
| def _compare_eq_sequence( | |||
| left: Sequence[Any], | |||
| right: Sequence[Any], | |||
| highlighter: _HighlightFunc, | |||
| highlighter: _HighlightFunc | Callable[[str], str], | |||
There was a problem hiding this comment.
This should not really be needed, given _HighlightFunc is compatible with Callable[[str], str]:
class _HighlightFunc(Protocol):
def __call__(self, source: str, lexer: Literal["diff", "python"] = "python") -> str:
"""Apply highlighting to the given source."""| @@ -501,10 +501,19 @@ def _compare_eq_dict( | |||
| if diff: | |||
| explanation += ["Differing items:"] | |||
| for k in diff: | |||
| left_val = left[k] | |||
| right_val = right[k] | |||
| if issequence(left_val) and issequence(right_val): | |||
There was a problem hiding this comment.
I'm not sure I like this approach, given this is really only specific for sequences inside dictionaries... this is not general because really we could have anything in there, for example other dicts or dataclasses, each with their own custom diff code... 🤔
The core of the original issue was the fact that it was truncating the diff. One idea is to not truncate anything when computing the diff, however truncation is there for a reason, to avoid dumping MBs of text in case of large data structures...
I'm a bit -1 on this solution, given it only fixes a very specific use case and makes the code slightly worse, but I don't have a definitive suggestion either.
There was a problem hiding this comment.
Yeah I initially thought we could take all the cases in _compare_eq_any and use those to create the new diffs but they seemed too long for cases outside of sequences due to their multi-line diffs, e.g.:
E assert {1, 2} == {1, 3}
E
E Extra items in the left set:
E 2
E Extra items in the right set:
E 3
Besides this though, I figured sequence comparisons would be the most likely use case, and it'd be worth having it there despite the bit of complexity. I could be wrong though!
There was a problem hiding this comment.
An alternative method might be to figure out a truncated diff for the other data types such as a simple Differing items for each instead of their specific diffs, but this seems overkill for just comparisons within dictionaries
|
Thanks a lot @reaganjlee for tackling this, but as I commented above I'm not sure this is an appropriate solution to this problem. Hopefully other maintainers can chime in. |
Fixes #11980
Let me know if there are any other suggestions for the message formats!