cohere 5.0.0 support #38465
cohere 5.0.0 support #38465
Conversation
potiuk
force-pushed
the
cohere-5-support
branch
2 times, most recently
from
0f2159d to
3173797
Compare
|
Seems that |
| self, texts: list[str], model: str = "embed-multilingual-v2.0" | ||
| ) -> list[list[float]]: | ||
| response = self.get_conn.embed(texts=texts, model=model) | ||
| ) -> list[list[float]] | cohere.EmbedByTypeResponseEmbeddings: |
There was a problem hiding this comment.
I think return type should be just EmbedResponse ref. Is there any case where we will still return list[list[float]]?
|
|
||
| - additional_query_parameters: typing.Dict[str, typing.Any]. A dictionary containing additional parameters to spread into the request's query parameters dict | ||
|
|
||
| - additional_body_parameters: typing.Dict[str, typing.Any]. A dictionary containing additional parameters to spread into the request's body parameters dict |
There was a problem hiding this comment.
It would be nice to point to original docs here, in case they change in future releases.
| return CohereHook( | ||
| conn_id=self.conn_id, | ||
| timeout=self.timeout, | ||
| max_retries=self.max_retries, |
There was a problem hiding this comment.
Should we use max_retries=self.max_retries in the same PR we are deprecating it? :)
| self.request_options = ( | ||
| {"max_retries": self.max_retries} | ||
| if self.request_options is None | ||
| else self.request_options.update({"max_retries": self.max_retries}) |
There was a problem hiding this comment.
nit: we generally use if/else in this manner to conditionally assign values, which is the intent of if clause here but in else you are updating the self.request_options dict. Which to me seems out of place. It would be much better to have a simplified version of the code here.
| self.request_options = ( | ||
| {"max_retries": self.max_retries} | ||
| if self.request_options is None | ||
| else self.request_options.update({"max_retries": self.max_retries}) |
There was a problem hiding this comment.
Looks like self.request_options will be None everytime in else case.
|
Also, we need to make sure the systems tests are working with these changes -
|
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
|
@okirialbert Do you wish to continue with this change? |
Yes I'll close it. I'll submit a PR for this with the required changes. |
github-actions
bot
removed
the
stale
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
github-actions
bot
added
the
stale
4 participants
This PR adds support for Cohere 5.0.0 update
Relates to #38349
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.