Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed ZeroDivisionError in JoinDocuments #7972

Merged
merged 35 commits into from
Jul 4, 2024
Merged

Conversation

nickprock
Copy link
Contributor

Related Issues

Proposed Changes:

I tried to use the method with few documents, in one case max and min have the same value.
I fixed it using:

for doc in documents:
    try:
        doc.score = (doc.score - min_score) / (max_score - min_score)
    except ZeroDivisionError:
        doc.score = 0  # if max and min is the same value all documents are poorly informative

I added also a test.

How did you test it?

unit tests

Notes for the reviewer

@shadeMe sorry I had not thought of this case when I developed the method.

Checklist

@nickprock nickprock requested a review from a team as a code owner July 3, 2024 14:15
@nickprock nickprock requested review from silvanocerza and removed request for a team July 3, 2024 14:15
@nickprock nickprock requested a review from a team as a code owner July 3, 2024 14:16
@nickprock nickprock requested review from dfokina and removed request for a team July 3, 2024 14:16
@coveralls
Copy link
Collaborator

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9779529574

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 89.967%

Files with Coverage Reduction New Missed Lines %
components/joiners/document_joiner.py 2 97.7%
Totals Coverage Status
Change from base Build 9777342637: -0.02%
Covered Lines: 6761
Relevant Lines: 7515

💛 - Coveralls

releasenotes/notes/release-note-9b2bc03a8a398078.yaml Outdated Show resolved Hide resolved
haystack/components/joiners/document_joiner.py Outdated Show resolved Hide resolved
test/components/joiners/test_document_joiner.py Outdated Show resolved Hide resolved
@shadeMe shadeMe added the ignore-for-release-notes PRs with this flag won't be included in the release notes. label Jul 3, 2024
@coveralls
Copy link
Collaborator

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9779557971

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 89.967%

Files with Coverage Reduction New Missed Lines %
components/joiners/document_joiner.py 2 97.7%
Totals Coverage Status
Change from base Build 9777342637: -0.02%
Covered Lines: 6761
Relevant Lines: 7515

💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9779860441

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 89.967%

Files with Coverage Reduction New Missed Lines %
components/joiners/document_joiner.py 2 97.7%
Totals Coverage Status
Change from base Build 9777342637: -0.02%
Covered Lines: 6761
Relevant Lines: 7515

💛 - Coveralls

haystack/components/joiners/document_joiner.py Outdated Show resolved Hide resolved
test/components/joiners/test_document_joiner.py Outdated Show resolved Hide resolved
@coveralls
Copy link
Collaborator

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9780178883

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 89.967%

Files with Coverage Reduction New Missed Lines %
components/joiners/document_joiner.py 2 97.7%
Totals Coverage Status
Change from base Build 9777342637: -0.02%
Covered Lines: 6761
Relevant Lines: 7515

💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9780227857

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 89.991%

Totals Coverage Status
Change from base Build 9777342637: 0.002%
Covered Lines: 6761
Relevant Lines: 7513

💛 - Coveralls

@nickprock nickprock requested a review from shadeMe July 3, 2024 15:21
@coveralls
Copy link
Collaborator

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9780675188

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 89.991%

Totals Coverage Status
Change from base Build 9777342637: 0.002%
Covered Lines: 6761
Relevant Lines: 7513

💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jul 3, 2024

Pull Request Test Coverage Report for Build 9783858554

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 89.991%

Totals Coverage Status
Change from base Build 9777342637: 0.002%
Covered Lines: 6761
Relevant Lines: 7513

💛 - Coveralls

@shadeMe shadeMe merged commit cafcf51 into deepset-ai:main Jul 4, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release-notes PRs with this flag won't be included in the release notes. topic:tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Distribution-based rank fusion in JoinDocuments
4 participants