-
Notifications
You must be signed in to change notification settings - Fork 128
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
feat: Add AnthropicGenerator and AnthropicChatGenerator #573
Conversation
9b79e07
to
122f56f
Compare
@julian-risch not ready yet, just trying to run some CI, please don't review yet |
We shouldn't use |
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.
Looks quite good. Very few small change requests. Not using inheritance is fine by me here.
markers = [ | ||
"unit: unit tests", | ||
"integration: integration tests", | ||
"generators: generators tests", |
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.
Let's add a marker for chat generators
"chat_generators: chat generator tests",
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.
And lets use the markers in the tests. Alternative would be to remove the "generators: generators tests",
line here.
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.
I'll remove them, I don't see the point of using them :-)
...s/anthropic/src/haystack_integrations/components/generators/anthropic/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
...s/anthropic/src/haystack_integrations/components/generators/anthropic/chat/chat_generator.py
Show resolved
Hide resolved
integrations/anthropic/src/haystack_integrations/components/generators/anthropic/generator.py
Show resolved
Hide resolved
"system", | ||
"max_tokens", | ||
"metadata", | ||
# "stream", explicitly passed to the interface |
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.
Let's just remove the line and if important mention it in docstrings.
...s/anthropic/src/haystack_integrations/components/generators/anthropic/chat/chat_generator.py
Outdated
Show resolved
Hide resolved
integrations/anthropic/example/documentation_rag_with_claude.py
Outdated
Show resolved
Hide resolved
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.
LGTM! 👍 The markers can be helpful for running a subset of the tests more easily. Here we already achieve separation through the two test files.
Why:
This Pull Request introduces a new integration for the Haystack framework, enabling the use of Anthropic's Claude family of language models. Anthropic's API provides advanced features such as streaming responses and customizability, making it an excellent addition to Haystack's list of supported integrations.
What:
The PR adds:
integrations/anthropic
including:src
directory containing the integration implementationtests
directory with test filespyproject.toml
defining build and packaging informationpydoc
directory with configuration and markdown file for automatically generated documentationLICENSE.txt
for the Apache License 2.0.github/workflows/anthropic.yml
file to add CI workflows for testing and packagingAnthropicChatGenerator
andAnthropicGenerator
for integrating with Anthropic's APIdocs
directorytests
directoryHow can it be used:
AnthropicChatGenerator
andAnthropicGenerator
components in your pipelines to generate responses and text using Anthropic's Claude family of modelsHow did you test it:
pytest
to validate the components' functionalities, such as their interaction with the Anthropic API, input and output handling, and the serialization and deserialization processesNotes for the reviewer:
ANTHROPIC_API_KEY
is setALLOWED_PARAMS
variable in both generator classes to confirm that the allowed parameters for Anthropic's API are up-to-date