Repository navigation
Conversation
Co-authored-by: Claude <noreply@anthropic.com>
Added fallback test cases for extra items in lists.
Slice comparison may raise or return non-bools for custom Sequences; fall back to the first-extra-item message instead of degrading the whole explanation. No known in-tree trigger. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…652-misleading-extra-item-message
OKAN-SOFTWARE
left a comment
There was a problem hiding this comment.
Reviewed commit 886f14b.
The updated message correctly handles cases where a single element was inserted in the middle of an otherwise matching sequence.
However, the new found_diff and len_diff == 1 condition does not distinguish a genuine insertion from a combination of differing values and unequal lengths.
For example:
left = [1, 2, 9]
right = [1, 3]The new logic reports:
At index 1 diff: 2 != 3
Left contains one more item: 2
But removing 2 produces [1, 9], which still differs from the right-hand sequence. There is no uniquely identifiable inserted element in this example.
The PR description mentions checking whether the remaining elements align after the first difference, but the current implementation only checks found_diff and the length difference.
Could we verify suffix alignment before identifying the first differing element as the extra item?
For example:
- When the left side is longer, check whether
left[i + 1:] == right[i:]. - When the right side is longer, check whether
right[i + 1:] == left[i:].
If the remaining elements don't align, retaining a more general length-difference message would avoid identifying an unsupported insertion.
A regression test containing both a length difference and unrelated mismatched values would help cover this distinction.
The focused selection logic was checked with the example above. GitHub's test workflow passes; I have not independently run the complete pytest suite.
|
Done. Thanks for your review. |
bee4ae0 to
5385e1a
Compare
|
Thanks for addressing my previous feedback. The updated suffix-alignment check resolves the issue I identified, and the added regression tests cover the problematic cases. The latest CI checks are passing. Looks good from my side! |
Prior art: #14286.
Pytest reported the wrong extra item when comparing sequences of different lengths. It printed the item past the end of the shorter side. The real extra item sat at the first differing index.
Before:
After:
The fix finds a single inserted item by checking whether the tails after the first differing index align, offset by one. It corrects only the single-extra-item case. Multi-item length differences still use the previous "first extra item" message.
Co-authored-bycommit trailers.changelogdirectory, with a name like<ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.AUTHORSin alphabetical order.