Skip to content
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[minor]:feat(embedding integration): modifying the Alibaba Tongyi chat model enableSearch parameter is invalid; add Alibaba Tongyi embedding #4662

Merged
merged 14 commits into from
Mar 12, 2024

Conversation

JoeABCDEF
Copy link
Contributor

@JoeABCDEF JoeABCDEF commented Mar 7, 2024

Fix the issue that the chatModel searchEnable parameter is invalid when only using community alibaba_tongyi;
Embedding with alibaba_tongyi integration

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Mar 7, 2024
Copy link

vercel bot commented Mar 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchainjs-api-refs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 12, 2024 6:15pm
langchainjs-docs ✅ Ready (Inspect) Visit Preview Mar 12, 2024 6:15pm

@dosubot dosubot bot added the auto:bug Related to a bug, vulnerability, unexpected error with an existing feature label Mar 7, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Mar 7, 2024
@JoeABCDEF JoeABCDEF changed the title modifying the enableSearch parameter is invalid modifying the enableSearch parameter is invalid; add Alibaba Tongyiqianwen embedding Mar 7, 2024
@JoeABCDEF JoeABCDEF changed the title modifying the enableSearch parameter is invalid; add Alibaba Tongyiqianwen embedding community[minor]: modifying the enableSearch parameter is invalid; add Alibaba Tongyiqianwen embedding Mar 7, 2024
@JoeABCDEF JoeABCDEF changed the title community[minor]: modifying the enableSearch parameter is invalid; add Alibaba Tongyiqianwen embedding community[minor]:feat(embedding integration): modifying the enableSearch parameter is invalid; add Alibaba Tongyiqianwen embedding Mar 7, 2024
@JoeABCDEF JoeABCDEF changed the title community[minor]:feat(embedding integration): modifying the enableSearch parameter is invalid; add Alibaba Tongyiqianwen embedding community[minor]:feat(embedding integration): modifying the Alibaba Tongyi chat model enableSearch parameter is invalid; add Alibaba Tongyi embedding Mar 8, 2024
@JoeABCDEF
Copy link
Contributor Author

Could you please reply, Does the code not meet the specification? Why can't it be merged?

request_id: string
}

export class AlibabaAIEmbeddings extends Embeddings implements AlibabaAIEmbeddingsParams {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, rename to AlibabaTongyiEmbeddingsParams

* The number of dimensions the resulting output embeddings should have.
* Only supported in `text-embedding-3` and later models.
*/
// dimensions?: number
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unused comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it

text_type: fieldsWithDefaults?.parameters?.text_type ?? "document"
}
// this.timeout = fieldsWithDefaults?.timeout
// this.dimensions = fieldsWithDefaults?.dimensions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented out code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it


/**
* Method to generate embeddings for an array of documents. Splits the
* documents into batches and makes requests to the OpenAI API to generate
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update OpenAI in docstring

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it

@jacoblee93
Copy link
Collaborator

Hey @JoeABCDEF, sorry for the delay, I have a lot to cover!

I don't think I can test this myself but the URL looks right and overall looks good, minus comments. Can you run yarn format and yarn lint and add a documentation page?

@jacoblee93 jacoblee93 added the close PRs that need one or two touch-ups to be ready label Mar 10, 2024
@JoeABCDEF
Copy link
Contributor Author

Hey @JoeABCDEF, sorry for the delay, I have a lot to cover!

I don't think I can test this myself but the URL looks right and overall looks good, minus comments. Can you run yarn format and yarn lint and add a documentation page?

I'm very sorry, a lot of things are not careful, but this function is normal that I have used it myself, and I will carefully change and resubmit the problem that occurs, I am very sorry

@jacoblee93
Copy link
Collaborator

Added docs + entrypoint from @langchain/community/embeddings/alibaba_tongyi - thank you!

@jacoblee93 jacoblee93 added lgtm PRs that are ready to be merged as-is and removed close PRs that need one or two touch-ups to be ready labels Mar 12, 2024
@jacoblee93 jacoblee93 merged commit df89898 into langchain-ai:main Mar 12, 2024
16 checks passed
@JoeABCDEF
Copy link
Contributor Author

JoeABCDEF commented Mar 13, 2024

Added docs + entrypoint from @langchain/community/embeddings/alibaba_tongyi - thank you!

Okay, I will, where is the documentation added?

@jacoblee93
Copy link
Collaborator

jacoblee93 commented Mar 13, 2024

Oh, I added it! It's live:

https://js.langchain.com/docs/integrations/text_embedding/alibaba_tongyi

It is finished :)

@JoeABCDEF
Copy link
Contributor Author

Oh, I added it! It's live:

https://js.langchain.com/docs/integrations/text_embedding/alibaba_tongyi

It is finished :)

thank you so much

@Jack-Z-Coding
Copy link

by the way ,how can you get the AlibabaTongyiEmbeddings key,when i visit "https://dashvector.console.aliyun.com/cn-hangzhou/overview" and get get key . its invalid
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:bug Related to a bug, vulnerability, unexpected error with an existing feature lgtm PRs that are ready to be merged as-is size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants