-
Notifications
You must be signed in to change notification settings - Fork 15.8k
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
community: add Needle retriever and document loader integration #28157
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
1f65a78
to
7280ef5
Compare
@efriis should be ok, Vercel deployment fails due to
I guess that is a FP? Should be all good with the PR. |
Hi Langchain team, @efriis 👋 This is Onur from Needle. We are waiting on this review for more than 2 months. There was another PR previously which we had to close and open this one since the review flow changed. Could you finalize reviewing this PR? Obviously we want to avoid same scenario happening again. Thanks for your efforts! |
hey sorry for the delay! would you be interested in integrating like this instead? then you don't have to deal with the "needs support" status for the integration, and docs PRs are much easier for us to review. Please take a look at the CI failures as well (looks like it's on the docs build, which would apply regardless of format of the integration!) https://python.langchain.com/docs/contributing/how_to/integrations/ |
actually merging in master to rebuild the docs - @JANHMS ' comment was addressed earlier, so it may fix itself |
Thank @efriis it did solve the issue by itself. I need this to be a part of the Are we ready to merge?
|
Hey @efriis sorry to bump again, but we actually need this asap and are waiting for a long time. It blocks us on another side. Please let's go ahead and merge so we have it available as the community package. |
1 similar comment
@eyurtsev Hi Eugene - sorry about chasing, but it's ready to be merged from our side. Can you take a look? We understand you guys are busy these days, we appreciate the effort! 🙏 |
PR title: "community: add Needle retriever and document loader integration"
PR message: Delete this entire checklist and replace with
docs/docs/integrations/retrievers/needle.ipynb
docs/docs/integrations/document_loaders/needle.ipynb
.needle-python
package is required as an external dependency for accessing Needle's API. It has been added to the extended testing dependencies list.Add tests and docs: If you're adding a new integration, please include
NeedleRetriever
andNeedleLoader
inlibs/community/tests/unit_tests
. These tests mock API calls to avoid relying on network access.docs/docs/integrations/
, showcasing both retriever and loader functionality.Lint and test: Run
make format
,make lint
, andmake test
from the root of the package(s) you've modified. See contribution guidelines for more: https://python.langchain.com/docs/contributing/make format
: Passedmake lint
: Passedmake test
: Passed (requiresneedle-python
to be installed locally; this package is not added to LangChain dependencies).Additional guidelines:
If no one reviews your PR within a few days, please @-mention one of baskaryan, efriis, eyurtsev, ccurme, vbarda, hwchase17.