Skip to content
New issue

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

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

Already on GitHub? # to your account

community: add top_k as param to Needle Retriever #29821

Merged
merged 6 commits into from
Feb 16, 2025

Conversation

JANHMS
Copy link
Contributor

@JANHMS JANHMS commented Feb 15, 2025

Thank you for contributing to LangChain!

  • PR title: "package: description"

    • Where "package" is whichever of langchain, community, core, etc. is being modified. Use "docs: ..." for purely docs changes, "infra: ..." for CI changes.
    • Example: "community: add foobar LLM"
  • PR message:
    This PR adds top_k as a param to the Needle Retriever. By default we use top 10.

  • Add tests and docs: If you're adding a new integration, please include

    1. a test for the integration, preferably unit tests that do not rely on network access,
    2. an example notebook showing its use. It lives in docs/docs/integrations directory.
  • Lint and test: Run make format, make lint and make test from the root of the package(s) you've modified. See contribution guidelines for more: https://python.langchain.com/docs/contributing/

Additional guidelines:

  • Make sure optional dependencies are imported within a function.
  • Please do not add dependencies to pyproject.toml files (even optional ones) unless they are required for unit tests.
  • Most PRs should not touch more than one package.
  • Changes should be backwards compatible.
  • If you are adding something to community, do not re-import it in langchain.

If no one reviews your PR within a few days, please @-mention one of baskaryan, efriis, eyurtsev, ccurme, vbarda, hwchase17.

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Feb 15, 2025
Copy link

vercel bot commented Feb 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 16, 2025 10:27am

@dosubot dosubot bot added community Related to langchain-community Ɑ: retriever Related to retriever module labels Feb 15, 2025
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Feb 15, 2025
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Feb 15, 2025
@JANHMS
Copy link
Contributor Author

JANHMS commented Feb 15, 2025

@efriis Hey man, adding a small detail here. Should be an easy to merge PR. Just adding the top_k to the Needle retriever.

@@ -45,6 +47,9 @@ class NeedleRetriever(BaseRetriever, BaseModel):
collection_id: Optional[str] = Field(
..., description="The ID of the Needle collection to search in"
)
top_k: Optional[int] = Field(
default=10, description="Maximum number of search results to return"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not set default as None, as in the client?

Won't this change behavior for existing users?

Copy link
Contributor Author

@JANHMS JANHMS Feb 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right. I will adjust, appreciate the comment.

@ccurme ccurme self-assigned this Feb 16, 2025
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Feb 16, 2025
@ccurme ccurme merged commit 60f58df into langchain-ai:master Feb 16, 2025
18 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
community Related to langchain-community lgtm PR looks good. Use to confirm that a PR is ready for merging. Ɑ: retriever Related to retriever module size:S This PR changes 10-29 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants