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.

Sign up for GitHub

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

Jump to bottom

community: support Hunyuan Embedding #23160

Merged
merged 21 commits into from
Dec 16, 2024
Merged

community: support Hunyuan Embedding #23160

merged 21 commits into from
Dec 16, 2024
+155 −0

Conversation

hcd233
Copy link
Contributor

@hcd233 hcd233 commented Jun 19, 2024 •
edited
Loading

description

  • I refactor Chathunyuan using tencentcloud sdk because I found the original one can't work in my application
  • I add HunyuanEmbeddings using tencentcloud sdk
  • Both of them are extend the basic class of langchain. I have fully tested them in my application

Dependencies

  • tencentcloud-sdk-python

@vercel Vercel
Copy link

vercel bot commented Jun 19, 2024 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 16, 2024 7:27pm

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 19, 2024
@dosubot dosubot bot added community Related to langchain-community Ɑ: embeddings Related to text embedding models module 🤖:refactor A large refactor of a feature(s) or restructuring of many files labels Jun 19, 2024
baskaryan
baskaryan reviewed Jun 19, 2024
View reviewed changes
libs/community/langchain_community/chat_models/hunyuan.py Outdated
Comment on lines 139 to 112
hunyuan_api_base: str = Field(default=DEFAULT_API_BASE)
"""Hunyuan custom endpoints"""
hunyuan_app_id: Optional[int] = None
"""Hunyuan App ID"""
hunyuan_secret_id: Optional[str] = None
"""Hunyuan Secret ID"""
hunyuan_secret_key: Optional[SecretStr] = None
"""Hunyuan Secret Key"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

changing the names of init args is a breaking change — do we need to make it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your reminder.
In addition to remove the hunyuan_api_base and hunyuan_app_id which maybe unused in newest hunyuan api,
maybe I should keep hunyuan_secret_id and hunyuan_secret_key instead of tencentcloud_secret_id and tencentcloud_secret_key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your reminder. In addition to remove the hunyuan_api_base and hunyuan_app_id which maybe unused in newest hunyuan api, maybe I should keep hunyuan_secret_id and hunyuan_secret_key instead of tencentcloud_secret_id and tencentcloud_secret_key.

I have completed the changes described above and tested it again ^^

@hcd233 hcd233 requested a review from baskaryan June 21, 2024 02:20
@vercel Vercel
Copy link

vercel bot commented Jun 22, 2024

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

@hcd233
Copy link
Contributor Author

hcd233 commented Jun 24, 2024

Hi! @baskaryan

I hope this message finds you well. I wanted to follow up on this merge request as it's been a while since the last update. I understand you might be busy, but I would greatly appreciate your feedback when you have a moment.

If there are any concerns or if you need any clarifications about the changes, please let me know. I'm happy to provide more context or make adjustments as needed.

Thank you for your time and attention. Looking forward to your review.

@baskaryan
Copy link
Collaborator

is this change still needed after #24172?

@hcd233
Copy link
Contributor Author

hcd233 commented Sep 3, 2024

is this change still needed after #24172?

I have read this PR in detail. Compared to this PR, Both of two PR implemented the hunyuan chat model with the Tencent Cloud SDK. In addition, I also implemented hunyuan embeddings. May be it will be helpful.

Here is the docs reference:
https://cloud.tencent.com/document/product/1729/102832

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Dec 14, 2024
@efriis efriis changed the title community(tencent hunyuan): Refactor ChatHunyuan and support Hunyuan Embedding community: support Hunyuan Embedding Dec 14, 2024
@efriis efriis enabled auto-merge (squash) December 14, 2024 02:03
@efriis efriis self-assigned this Dec 14, 2024
@efriis efriis merged commit 5c17a4a into langchain-ai:master Dec 16, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efriis efriis efriis approved these changes

@baskaryan baskaryan Awaiting requested review from baskaryan

Assignees

@efriis efriis

Labels
community Related to langchain-community Ɑ: embeddings Related to text embedding models module lgtm PR looks good. Use to confirm that a PR is ready for merging. 🤖:refactor A large refactor of a feature(s) or restructuring of many files size:L This PR changes 100-499 lines, ignoring generated files.
Projects
PR Reviews
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hcd233 @baskaryan @efriis @ccurme