feat: CohereGenerator #6034

sunilkumardash9 · 2023-10-12T09:08:55Z

Related Issues

fixes CohereGenerator #5984

Proposed Changes:

CohereGenerator to generate Cohere LLM responses from user-provided prompts

How did you test it?

Unit tests

Checklist

I have read the contributors guidelines and the code of conduct
I have updated the related issue with new insights and changes
I added unit tests and updated the docstrings
I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test:.
I documented my code
I ran pre-commit hooks and fixed any issue

Signed-off-by: sunilkumardash9 <[email protected]>

CLAassistant · 2023-10-12T09:09:03Z

All committers have signed the CLA.

ZanSara · 2023-10-13T11:14:16Z

Hello @sunilkumardash9! I'm going to take care of the review soon. In the meantime, let's fix the CI. Have you followed the installation instructions in our contributions guidelines? You need to install pre-commit hooks for the CI to run properly your tests.

On top of that, after you installed the pre-commit hooks you will need to run pre-commit run --all. It's needed just one time, to fix all the issues that were committed before installing the hooks.

Let me know if you have issues!

sunilkumardash9 · 2023-10-13T12:06:34Z

Hi @ZanSara, I followed what you suggested. I got two failed checks for Ruff and Codespell. Is it a problem? Though running hooks individually runs fine.

2. removed commented files in test-cohere_generators 3. removed unused imports Signed-off-by: sunilkumardash9 <[email protected]>

…ohere-generator

ZanSara · 2023-10-13T14:13:53Z

Hey @sunilkumardash9 , if Ruff and Codespell don't pass that means that there are some small issues with your code. Normally the error messages explain what's the issue and how to fix it, and if it's not clear they should be easy to Google. If you struggle with some of them you can share them here and I can help interpreting them.

ZanSara

A few notes

haystack/preview/components/generators/cohere/cohere.py

2. remove dict casting of metadata in run Signed-off-by: sunilkumardash9 <[email protected]>

Signed-off-by: sunilkumardash9 <[email protected]>

ZanSara · 2023-10-13T15:42:59Z

@sunilkumardash9 I see that the CI is now failing because cohere is not installed. Please add cohere to the dependencies installed at this line:

haystack/.github/workflows/tests_preview.yml

Line 119 in aaee03a

    
           run: pip install .[dev,preview] langdetect transformers[torch,sentencepiece]==4.32.1 'sentence-transformers>=2.2.0' pypdf openai-whisper tika 'azure-ai-formrecognizer>=3.2.0b2'

It should fix the issue.

You will need to do the same thing here:

haystack/.github/workflows/linting_preview.yml

Line 40 in aaee03a

    
           run: pip install .[dev,preview] langdetect transformers[torch,sentencepiece]==4.32.1 'sentence-transformers>=2.2.0' pypdf openai-whisper tika 'azure-ai-formrecognizer>=3.2.0b2'

to also make PyLint pass.

Signed-off-by: sunilkumardash9 <[email protected]>

2. small change in doc string Signed-off-by: sunilkumardash9 <[email protected]>

ZanSara · 2023-10-16T08:57:08Z

@sunilkumardash9 I see there's typo in the dependencies list: cohere should be outside of the quotes, not inside. That should fix the CI.

2. changed api key env var from CO_API_KEY to COHERE_API_KEY Signed-off-by: sunilkumardash9 <[email protected]>

ZanSara · 2023-10-16T10:21:24Z

@sunilkumardash9 You can ignore the MyPy issue for now (we're working on it, it's a larger issue), while for the others, I forgot to make you update these lines as well:

haystack/.github/workflows/linting_preview.yml

Line 74 in aaee03a

    
                     pip install .[dev,preview] langdetect transformers[torch,sentencepiece]==4.32.1 'sentence-transformers>=2.2.0' pypdf openai-whisper tika 'azure-ai-formrecognizer>=3.2.0b2'

haystack/.github/workflows/tests_preview.yml

Line 178 in aaee03a

    
           run: pip install .[dev,preview] langdetect transformers[torch,sentencepiece]==4.32.1 'sentence-transformers>=2.2.0' pypdf openai-whisper tika 'azure-ai-formrecognizer>=3.2.0b2'

haystack/.github/workflows/tests_preview.yml

Line 233 in aaee03a

    
           run: pip install .[dev,preview] langdetect transformers[torch,sentencepiece]==4.32.1 'sentence-transformers>=2.2.0' pypdf openai-whisper tika 'azure-ai-formrecognizer>=3.2.0b2'

haystack/.github/workflows/tests_preview.yml

Line 285 in aaee03a

    
           run: pip install .[dev,preview] langdetect transformers[torch,sentencepiece]==4.32.1 'sentence-transformers>=2.2.0' pypdf openai-whisper tika 'azure-ai-formrecognizer>=3.2.0b2'

The change is the same, you just need to add cohere at the end.

Don't forget to do git pull, by the way: I updated your branch to try fixing the mypy issue but it didn't work 😅

Signed-off-by: sunilkumardash9 <[email protected]>

sunilkumardash9 · 2023-10-16T10:51:48Z

@ZanSara, I should have scrolled down a bit too. 😅

ZanSara

A few small fixes, overall looking good! I believe we will manage to merge this one soon

ZanSara · 2023-10-19T15:34:57Z

haystack/preview/components/generators/cohere/cohere.py

+        Args:
+            api_key (str): The API key for the Cohere API.
+            model_name (str): The name of the model to use.
+            streaming_callback (Callable, optional): A callback function to be called with the streaming response. Defaults to None.
+            api_base_url (str, optional): The base URL of the Cohere API. Defaults to "https://api.cohere.ai".


We use a different style for docstrings: it's quite important for our automated API documentation tooling, so let's fix it.

Here is how it should look like:

Suggested change

Args:

api_key (str): The API key for the Cohere API.

model_name (str): The name of the model to use.

streaming_callback (Callable, optional): A callback function to be called with the streaming response. Defaults to None.

api_base_url (str, optional): The base URL of the Cohere API. Defaults to "https://api.cohere.ai".

Instantiates a `CohereGenerator` component.

:param api_key: The API key for the Cohere API.

:param model_name: The name of the model to use.

:param streaming_callback: A callback function to be called with the streaming response. Defaults to None.

:param api_base_url: The base URL of the Cohere API. Defaults to "https://api.cohere.ai".

:param kwargs: additional model parameters. These will be used during generation.

In addition:

for model_name, let's give some example model names.

for kwargs, let's add a link to the Cohere documentation where these arguments are listed.

I was thinking the same, should have stuck with the way it is in GptGenerator. For the Kwargs, I have added the params in doc_string.

ZanSara · 2023-10-19T15:36:22Z

haystack/preview/components/generators/cohere/cohere.py

+        replies: List[str]
+        metadata: List[Dict[str, Any]]


Are you sure we need these type definitions? Does mypy raises errors if you remove these two lines?

ZanSara · 2023-10-19T15:39:37Z

haystack/preview/components/generators/cohere/cohere.py

+        metadata = [{"finish_reason": response[0].finish_reason}]
+        replies = [response[0].text]


If users want to receive multiple alternative responses from the model, this won't work. Let's leverage the fact that we can return a list:

Suggested change

metadata = [{"finish_reason": response[0].finish_reason}]

replies = [response[0].text]

metadata = [{"finish_reason": resp.finish_reason} for resp in response]

replies = [resp.text for resp in response]

ZanSara · 2023-10-19T15:41:20Z

haystack/preview/components/generators/cohere/cohere.py

+        api_key: str,
+        model: str = "command",
+        streaming_callback: Optional[Callable] = None,
+        api_base_url: str = API_BASE_URL,


In the tests you're using cohere.COHERE_API_URL. Can we use the same here instead of duplicating its content into a constant? In this way, if cohere updates this value we won't need to do anything and the component will still work.

ZanSara · 2023-10-19T15:42:56Z

haystack/preview/components/generators/cohere/cohere.py

+def default_streaming_callback(chunk):
+    """
+    Default callback function for streaming responses from Cohere API.
+    Prints the tokens of the first completion to stdout as soon as they are received and returns the chunk unchanged.
+    """
+    print(chunk.text, flush=True, end="")


I think we can remove this function, because it's used only in the tests. To test for a streaming callback, you can define it in the tests themselves.

2. Added kwargs doc strings for CohereGenerator 3. removed type hints for metadata and replies 4. use COHERE_API_URL instead of hard coded URL. Signed-off-by: sunilkumardash9 <[email protected]>

dfokina

Hi @sunilkumardash9 ! Adding some cosmetic docstrings suggestions.

haystack/preview/components/generators/cohere/cohere.py

Co-authored-by: Daria Fokina <[email protected]>

ZanSara · 2023-11-23T10:55:51Z

Hello @sunilkumardash9, sorry for the late feedback. I'm going to take over this PR in order to fix the tests and merge it. Thank you for your contribution!

julian-risch · 2023-11-23T12:56:43Z

Closing this PR in favor of #6395

added CohereGenerator with unit tests

0cbf196

Signed-off-by: sunilkumardash9 <[email protected]>

sunilkumardash9 requested a review from a team as a code owner October 12, 2023 09:08

sunilkumardash9 requested review from ZanSara and removed request for a team October 12, 2023 09:08

github-actions bot added topic:tests type:documentation Improvements on the docs labels Oct 12, 2023

sunilkumardash9 and others added 2 commits October 12, 2023 14:39

Merge branch 'main' into add-cohere-generator

3576a4a

Merge branch 'main' into add-cohere-generator

2f48f60

sunilkumardash9 changed the title ~~added CohereGenerator with unit tests~~ added CohereGenerator Oct 13, 2023

sunilkumardash9 changed the title ~~added CohereGenerator~~ feat: CohereGenerator Oct 13, 2023

sunilkumardash9 and others added 3 commits October 13, 2023 18:05

1. added releasenote

bdaf1c2

2. removed commented files in test-cohere_generators 3. removed unused imports Signed-off-by: sunilkumardash9 <[email protected]>

Merge branch 'main' into add-cohere-generator

b518339

Merge remote-tracking branch 'origin/add-cohere-generator' into add-c…

e2547aa

…ohere-generator

sunilkumardash9 requested a review from a team as a code owner October 13, 2023 12:43

sunilkumardash9 requested review from dfokina and removed request for a team October 13, 2023 12:43

ZanSara reviewed Oct 13, 2023

View reviewed changes

haystack/preview/components/generators/cohere/cohere.py Outdated Show resolved Hide resolved

haystack/preview/components/generators/cohere/cohere.py Outdated Show resolved Hide resolved

sunilkumardash9 added 2 commits October 13, 2023 20:09

1. move client creation to __init__

127e2c7

2. remove dict casting of metadata in run Signed-off-by: sunilkumardash9 <[email protected]>

few fixes

bcd855e

Signed-off-by: sunilkumardash9 <[email protected]>

add cohere to git workflows

3fa2f0d

Signed-off-by: sunilkumardash9 <[email protected]>

github-actions bot added the topic:CI label Oct 13, 2023

1. CohereGenerator as top level import in generators

f58e602

2. small change in doc string Signed-off-by: sunilkumardash9 <[email protected]>

ZanSara self-requested a review October 16, 2023 08:57

1. corrected git workflow files for cohere import

e074a5e

2. changed api key env var from CO_API_KEY to COHERE_API_KEY Signed-off-by: sunilkumardash9 <[email protected]>

Merge branch 'main' into add-cohere-generator

44a3f14

added cohere in missed out workflow installs

106e7fc

Signed-off-by: sunilkumardash9 <[email protected]>

Merge branch 'main' into add-cohere-generator

3b4574d

ZanSara suggested changes Oct 19, 2023

View reviewed changes

1. Removed default_streaming_callback from cohere.py and added in test.

4ece817

2. Added kwargs doc strings for CohereGenerator 3. removed type hints for metadata and replies 4. use COHERE_API_URL instead of hard coded URL. Signed-off-by: sunilkumardash9 <[email protected]>

dfokina reviewed Oct 20, 2023

View reviewed changes

sunilkumardash9 and others added 5 commits October 20, 2023 23:12

Update haystack/preview/components/generators/cohere/cohere.py

bb63179

Co-authored-by: Daria Fokina <[email protected]>

Update haystack/preview/components/generators/cohere/cohere.py

35a1304

Co-authored-by: Daria Fokina <[email protected]>

Update haystack/preview/components/generators/cohere/cohere.py

24359a0

Co-authored-by: Daria Fokina <[email protected]>

Update haystack/preview/components/generators/cohere/cohere.py

6449bd2

Co-authored-by: Daria Fokina <[email protected]>

Update haystack/preview/components/generators/cohere/cohere.py

b43c698

Co-authored-by: Daria Fokina <[email protected]>

masci added the 2.x Related to Haystack v2.0 label Oct 30, 2023

masci assigned ZanSara Nov 23, 2023

ZanSara and others added 4 commits November 23, 2023 09:45

Merge branch 'main' into add-cohere-generator

282ebab

move out of folder

e51d787

Merge branch 'main' into add-cohere-generator

2378802

black

1d8c849

ZanSara mentioned this pull request Nov 23, 2023

feat: CohereGenerator #6395

Merged

julian-risch closed this Nov 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: CohereGenerator #6034

feat: CohereGenerator #6034

sunilkumardash9 commented Oct 12, 2023

CLAassistant commented Oct 12, 2023 •

edited

Loading

ZanSara commented Oct 13, 2023

sunilkumardash9 commented Oct 13, 2023

ZanSara commented Oct 13, 2023

ZanSara left a comment

ZanSara commented Oct 13, 2023 •

edited

Loading

ZanSara commented Oct 16, 2023

ZanSara commented Oct 16, 2023

sunilkumardash9 commented Oct 16, 2023

ZanSara left a comment

ZanSara Oct 19, 2023

sunilkumardash9 Oct 19, 2023

ZanSara Oct 19, 2023

ZanSara Oct 19, 2023

ZanSara Oct 19, 2023

ZanSara Oct 19, 2023

dfokina left a comment

ZanSara commented Nov 23, 2023

julian-risch commented Nov 23, 2023

-        Args:
-            api_key (str): The API key for the Cohere API.
-            model_name (str): The name of the model to use.
-            streaming_callback (Callable, optional): A callback function to be called with the streaming response. Defaults to None.
-            api_base_url (str, optional): The base URL of the Cohere API. Defaults to "https://api.cohere.ai".
+        Instantiates a `CohereGenerator` component.
+        :param api_key: The API key for the Cohere API.
+        :param model_name: The name of the model to use.
+        :param streaming_callback: A callback function to be called with the streaming response. Defaults to None.
+        :param api_base_url: The base URL of the Cohere API. Defaults to "https://api.cohere.ai".
+        :param kwargs: additional model parameters. These will be used during generation.

		metadata = [{"finish_reason": response[0].finish_reason}]
		replies = [response[0].text]

feat: CohereGenerator #6034

feat: CohereGenerator #6034

Conversation

sunilkumardash9 commented Oct 12, 2023

Related Issues

Proposed Changes:

How did you test it?

Checklist

CLAassistant commented Oct 12, 2023 • edited Loading

ZanSara commented Oct 13, 2023

sunilkumardash9 commented Oct 13, 2023

ZanSara commented Oct 13, 2023

ZanSara left a comment

Choose a reason for hiding this comment

ZanSara commented Oct 13, 2023 • edited Loading

ZanSara commented Oct 16, 2023

ZanSara commented Oct 16, 2023

sunilkumardash9 commented Oct 16, 2023

ZanSara left a comment

Choose a reason for hiding this comment

ZanSara Oct 19, 2023

Choose a reason for hiding this comment

sunilkumardash9 Oct 19, 2023

Choose a reason for hiding this comment

ZanSara Oct 19, 2023

Choose a reason for hiding this comment

ZanSara Oct 19, 2023

Choose a reason for hiding this comment

ZanSara Oct 19, 2023

Choose a reason for hiding this comment

ZanSara Oct 19, 2023

Choose a reason for hiding this comment

dfokina left a comment

Choose a reason for hiding this comment

ZanSara commented Nov 23, 2023

julian-risch commented Nov 23, 2023

CLAassistant commented Oct 12, 2023 •

edited

Loading

ZanSara commented Oct 13, 2023 •

edited

Loading