langchain icon indicating copy to clipboard operation
langchain copied to clipboard

community: add support for sentence window retrieval in Qdrant.py by enabling integer ids when using Qdrant.from_texts(....)

Open sagarvadodaria opened this issue 9 months ago • 2 comments

  • [ ] PR message:

    • Description: This PR is a prerequisite for a future PR (i will share shortly) about sentence window retrieval using Qdrant. In this PR we are just making the parameter 'ids' in the methods like 'Qdrant.from_texts()' to accept both list of string / int as it is supported by Qdrant itself.
    • Issue: Didn't create any issues for this feature and i did not find existing issues either.
    • Dependencies: None
    • Twitter handle: @sagar_vadodaria
  • [ ] Add tests and docs:

    1. added integration test based on existing tests that use string based UUIDs,
    2. Updated the example notebook (docs/docs/integrations/vectorstores/qdrant.ipynb) showing its use.
  • [ ] Lint and test:

  1. Run make format ==> No errors
  2. make lint ==> No errors
  3. make test ===> No errors

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

sagarvadodaria avatar May 10 '24 23:05 sagarvadodaria

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 May 20, 2024 3:38pm

vercel[bot] avatar May 10 '24 23:05 vercel[bot]

@baskaryan / @efriis / @eyurtsev , just wanted to bring this PR to your attention. This adds a new chunk_window_retrieval_search for Qdrant which is based on sentence window.

sagarvadodaria avatar May 15 '24 19:05 sagarvadodaria

@baskaryan / @efriis / @eyurtsev , just wanted to bring this PR to your attention. This adds a new chunk_window_retrieval_search for Qdrant which is based on sentence window.

@baskaryan / @efriis / @eyurtsev

Hello again, just wanted to check if we can hear your thoughts about this, so that we can plan accordingly. Do you agree with this approach or this has no chance. Either way, it would help us decide our design. At the bare minimum we should add the support for adding Integers as PointIds in Qdrant. At this point, Qdrant.from_texts(.._) method only accepts List of String. Any reason not to accept List of Integers as well since Qdrant does support that?

sagarvadodaria avatar May 20 '24 15:05 sagarvadodaria

Thank you for the PR. This PR is marked Needs Support and has not yet received the 5 upvotes required by maintainers for review. It has been open for at least 25 days. Per the LangChain review process, this PR will be closed in 5 days if it does not reach the required threshold.

The Needs Support status is intended to prioritize review time on features that have demonstrated community support. If you feel this status was assigned in error or need more time to gather the required upvotes, please ping (at)ccurme and (at)efriis.

langcarl[bot] avatar Nov 04 '24 19:11 langcarl[bot]