-
Notifications
You must be signed in to change notification settings - Fork 121
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
Add rerank document compressor #331
Conversation
df8c35a
to
70f4e2d
Compare
Hi @jpfcabral, interesting contribution! I noticed that the default region is set to |
Fair point, @mgvalverde , I just changed on bbc0243 |
@jpfcabral |
@3coins Following on the documentation you sent me, I need to call it by bedrock-agent-runtime so we can configure the request with Note: bedrock-agent-runtime rerank only support requests with I just added a commit 13ad88f with the changes above, let me know if need some fixes on that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpfcabral
Thanks for submitting this PR and a quick turnaround on the updates.
Great job on adding the examples in the PR description, it might be more useful to add a notebook with those samples in the samples/document_compressors
directory.
Also, to keep the module organization consistent with community, does it sound better to put this under document_compressors
rather than rerank
?
@jpfcabral |
@3coins The lint issues have been resolved. I also refactored d69a71f the initialize_client method to align with the pattern used in other langchain libraries, following this reference. I agree that the nextTokens implementation should be handled in a separate PR—let me know if I can help with that in any way. Thanks for the review! |
7194759
to
e3de880
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpfcabral
Great job on getting the updates done. LGTM! 🚀
@jpfcabral |
Done! |
Fixes #298
Added:
from langchain_aws import BedrockRerank
Some snippets: