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

BUG: Correct the input bytes data by langchain_openai #2589 #2600

Merged
merged 2 commits into from
Nov 29, 2024

Conversation

xiyuan-lee
Copy link
Contributor

No description provided.

@XprobeBot XprobeBot added the bug Something isn't working label Nov 28, 2024
@XprobeBot XprobeBot added this to the v1.x milestone Nov 28, 2024
@qinxuye
Copy link
Contributor

qinxuye commented Nov 28, 2024

Lint failed, please install pre-commit and run pre-commit install at the project directory, this will do pre-check before committing.

@xiyuan-lee
Copy link
Contributor Author

what's wrong?

2024-11-28 16:40:53.802 [info] An error has occurred: InvalidManifestError:
==> File /home/oem/.cache/pre-commit/repoq8t72eld/.pre-commit-hooks.yaml
==> At Hook(id='black')
==> At key: stages
==> At index 0
=====> Expected one of commit, commit-msg, manual, merge-commit, post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, push but got: 'pre-commit'
Check the log at /home/oem/.cache/pre-commit/pre-commit.log

@qinxuye
Copy link
Contributor

qinxuye commented Nov 28, 2024

what's wrong?

2024-11-28 16:40:53.802 [info] An error has occurred: InvalidManifestError: ==> File /home/oem/.cache/pre-commit/repoq8t72eld/.pre-commit-hooks.yaml ==> At Hook(id='black') ==> At key: stages ==> At index 0 =====> Expected one of commit, commit-msg, manual, merge-commit, post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, push but got: 'pre-commit' Check the log at /home/oem/.cache/pre-commit/pre-commit.log

You need to cd the xinference directory, there is a config file of pre-commit.

@qinxuye
Copy link
Contributor

qinxuye commented Nov 28, 2024

I just found this part is implemented in rest API, maybe it's better to put them into the embedding model?

def create_embedding(self, sentences: Union[str, List[str]], **kwargs):

Since rest API may not be in charge of any computation including the decoding.

@xiyuan-lee
Copy link
Contributor Author

I just found this part is implemented in rest API, maybe it's better to put them into the embedding model?

def create_embedding(self, sentences: Union[str, List[str]], **kwargs):

Since rest API may not be in charge of any computation including the decoding.

Ok, I move the changes to the embedding model.

@xiyuan-lee xiyuan-lee force-pushed the main branch 2 times, most recently from 6d3255a to c08fd71 Compare November 28, 2024 17:04
@xiyuan-lee xiyuan-lee force-pushed the main branch 3 times, most recently from fb504f4 to 7907284 Compare November 29, 2024 04:56
@qinxuye
Copy link
Contributor

qinxuye commented Nov 29, 2024

I slightly modified your code, check all the data if all of them is int is not so necessary IMHO, I think we could only check if it's list of list of int, and check the first int is OK, or this check may cost too much CPU.

Copy link
Contributor

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

@qinxuye qinxuye merged commit f4b5b42 into xorbitsai:main Nov 29, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants