-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[ENH] Add hosted splade embedding function to python and js #5610
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
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
bd0d7b2
to
3d3426a
Compare
Add Hosted Splade Embedding Functionality for Python and JS Clients This pull request introduces a new hosted sparse embedding function for the Splade model family, enabling users to compute Splade-based sparse vectors using the Chroma hosted embedding service via both Python and JavaScript clients. It adds fully functional implementations, API schema definitions, and integration hooks for the new embedding function, ensuring unified support, configuration, validation, and documentation across both client ecosystems. Substantial updates also include dependency and package management, registration and retrieval logic for sparse embedding functions, and comprehensive test coverage for core functionality and error conditions. Key Changes• Introduced new Affected Areas• This summary was automatically generated by @propel-code-bot |
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Outdated
Show resolved
Hide resolved
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Outdated
Show resolved
Hide resolved
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Show resolved
Hide resolved
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Show resolved
Hide resolved
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Show resolved
Hide resolved
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Outdated
Show resolved
Hide resolved
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Show resolved
Hide resolved
3d3426a
to
53968a0
Compare
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Show resolved
Hide resolved
53968a0
to
20bcadf
Compare
20bcadf
to
6515339
Compare
} | ||
|
||
try: | ||
import httpx |
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.
[BestPractice]
Redundant httpx import inside method. The httpx library is already imported and validated in the constructor (line 32). The import on line 85 inside the __call__
method is unnecessary and could impact performance with repeated calls.
import httpx | |
response = self._session.post(self._api_url, json=payload, timeout=60) |
Note: Reusing the existing httpx.Client instance (self._session) is more efficient as it leverages connection pooling and avoids redundant imports.
⚡ Committable suggestion
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Context for Agents
[**BestPractice**]
Redundant httpx import inside method. The httpx library is already imported and validated in the constructor (line 32). The import on line 85 inside the `__call__` method is unnecessary and could impact performance with repeated calls.
```suggestion
response = self._session.post(self._api_url, json=payload, timeout=60)
```
*Note: Reusing the existing httpx.Client instance (self._session) is more efficient as it leverages connection pooling and avoids redundant imports.*
⚡ **Committable suggestion**
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
File: chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Line: 85
chromadb/utils/embedding_functions/chroma_cloud_splade_embedding_function.py
Show resolved
Hide resolved
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!
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
Manually tested both embedding functions, work as intended
pytest
for python,yarn test
for js,cargo test
for rustMigration plan
Are there any migrations, or any forwards/backwards compatibility changes needed in order to make sure this change deploys reliably?
Observability plan
What is the plan to instrument and monitor this change?
Documentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?