fix: preserve colliding metadata keys in Document.__eq__#11982
fix: preserve colliding metadata keys in Document.__eq__#11982NIK-TIGER-BILL wants to merge 1 commit into
Conversation
…11969) Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
|
Someone is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
NIK-TIGER-BILL seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
anakin87
left a comment
anakin87
left a comment
There was a problem hiding this comment.
Please make the CLA check pass: you might need to rewrite the Git history.
I also left some comments.
Ping me when ready.
| doc3 = Document(content="hello", meta={"score": 0.5}) | ||
| doc4 = Document(content="hello", meta={"score": 0.9}) | ||
| assert doc3 != doc4 | ||
|
|
||
| doc5 = Document(content="hello", meta={"content": "from_meta"}) | ||
| doc6 = Document(content="hello", meta={"content": "also_from_meta_but_different"}) | ||
| assert doc5 != doc6 |
There was a problem hiding this comment.
| doc3 = Document(content="hello", meta={"score": 0.5}) | |
| doc4 = Document(content="hello", meta={"score": 0.9}) | |
| assert doc3 != doc4 | |
| doc5 = Document(content="hello", meta={"content": "from_meta"}) | |
| doc6 = Document(content="hello", meta={"content": "also_from_meta_but_different"}) | |
| assert doc5 != doc6 | |
| doc3 = Document(id="same_id", content="hello", meta={"score": 0.5}) | |
| doc4 = Document(id="same_id", content="hello", meta={"score": 0.9}) | |
| assert doc3 != doc4 | |
| doc5 = Document(id="same_id", content="hello", meta={"content": "from_meta"}) | |
| doc6 = Document(id="same_id", content="hello", meta={"content": "different_from_meta"}) | |
| assert doc5 != doc6 |
|
|
||
| def test_equality_preserves_meta_keys_colliding_with_top_level_fields(): | ||
| """ | ||
| Regression test for #11969. |
There was a problem hiding this comment.
- no need to mention the issue
- shorter docstring explanation
|
|
||
| def test_equality_with_colliding_meta_keys_when_meta_actually_matches(): | ||
| """ | ||
| Companion to the regression test for #11969. |
anakin87
added
the
cla-pending
|
Hi @NIK-TIGER-BILL, thanks a lot for your contribution! 🙏 We noticed that the Contributor License Agreement (CLA) check ( To get your PR reviewed, please sign the CLA via the link in the |
|
Hi @NIK-TIGER-BILL, just a friendly reminder: this PR is still in draft because the Contributor License Agreement (CLA) hasn't been signed yet. We'd love to review your contribution! Please sign the CLA via the link in the |
4 participants
Related Issues
Proposed Changes:
previously called , and defaults to . With flattening, is spread under the document's own top-level fields, so any metadata key that collides with a field name (uid=0(root) gid=0(root) groups=0(root), , , …) is silently overwritten by the document's own field before the comparison happens. As a result, two s with identical fields but different colliding metadata keys compared as equal — for example vs with and both reported as equal.
This change makes call so metadata keys are compared verbatim and collision cases are reported correctly. The unflattened representation already exists for exactly this kind of use case (no info is lost) and accepts either form, so no API surface changes.
How did you test it?
Added three regression tests in :
All checks verified locally by importing the dataclass module directly with stubbed package hierarchy (full env not available in this sandbox).
Notes for the reviewer
This is intentionally a minimal one-line semantic change with no API surface change. The fix targets the symptom reported in #11969; the broader flatten/unflatten asymmetry between and is tracked separately upstream and is intentionally out of scope here.
Checklist
fix:.releasenotes/notes/.