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

DocumentJoiner component's run method doesn't accept a top_k value #7702

Closed
1 task done
ms130 opened this issue May 16, 2024 · 1 comment · Fixed by #7709
Closed
1 task done

DocumentJoiner component's run method doesn't accept a top_k value #7702

ms130 opened this issue May 16, 2024 · 1 comment · Fixed by #7709
Assignees
Labels
Contributions wanted! Looking for external contributions good first issue Good for newcomers P3 Low priority, leave it in the backlog

Comments

@ms130
Copy link

ms130 commented May 16, 2024

Describe the bug
The DocumentJoiner component's run method doesn't allow you to pass a value for the top_k parameter, see here. If you try to do so, you'll get the following: ValueError: Input top_k not found in component joiner.

This means you can only pass a value for top_k when you initialize the component, and so you can't give it a value e.g. at query time using pipe.run("DocumentJoiner": {"top_k": top_k}) if you use it in a query pipeline.

Expected behavior
Providing this component with a top_k value in a pipeline using e.g. pipe.run("DocumentJoiner": {"top_k": top_k}) should not raise a ValueError.

A simple change to the run method, adding top_k as an argument, and replacing these lines inside the function with the following should fix it:

if top_k:
    output_documents = output_documents[: top_k]
elif self.top_k:
    output_documents = output_documents[: self.top_k]

To Reproduce
Create a query pipeline containing a DocumentJoiner and run it using pipe.run("DocumentJoiner": {"top_k": top_k}).

FAQ Check

@anakin87
Copy link
Member

Hey, @ms130...

This sounds like a simple and useful improvement.

Feel free to open a PR!

@anakin87 anakin87 added P3 Low priority, leave it in the backlog Contributions wanted! Looking for external contributions good first issue Good for newcomers labels May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contributions wanted! Looking for external contributions good first issue Good for newcomers P3 Low priority, leave it in the backlog
Projects
Development

Successfully merging a pull request may close this issue.

3 participants