-
Notifications
You must be signed in to change notification settings - Fork 3
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
[VEC-410] Update python client used in example applications #65
Conversation
Would you merge main? I think there are changes in here from a branch that recently got merged. |
Resolved the merge conflict. Did you mean resolving merge conflict or something else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one question regarding dependencies but lgtm. I think we will have to merge this before #63
@@ -12,7 +12,7 @@ waitress | |||
|
|||
# Quote processing | |||
numpy==1.26.4 # fix issues in dependencies after 2.0.0 release | |||
sentence-transformers==2.2.2 | |||
sentence-transformers==3.2.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any idea why we were using https://github.com/UKPLab/sentence-transformers/releases/tag/v2.2.2 before? This doesn't break anything we are using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked into git history.
When this work was done (10/11/2023); the 2.2.2 was the latest version of sentence-transformers
package. My guess is, this is the reason that specific version(2.2.2) was used then.
link to pypi release history: https://pypi.org/project/sentence-transformers/#history
On the breaking aspects:
I think we will have to go to every application and ensure the versions are updated everywhere. While debugging came across very recent issues
- cannot import name 'cached_download' from 'huggingface_hub' easydiffusion/easydiffusion#1851
The impression in the community is; hugging face have restructured lot of urls, which is breaking things, thus we will have to check everywhere and update.
I had seen changes from the other branch. Fixed with merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested prism only, and reviewed manually. Approved.
sentence-transformers
to 3.2.1 ( was gettingcannot import name 'cached_download' from 'huggingface_hub'
)