Conversation
jeffchuber
left a comment
There was a problem hiding this comment.
thanks for this!!! added a very clarification requests/comments
| port: Optional[int] = None | ||
| ssl: Optional[bool] = False | ||
| n_results: Optional[int] = 4 | ||
| embedding_function: Optional[Any] = None |
There was a problem hiding this comment.
should this be typed? also im thinking it shouldnt be optional?
| host: Optional[str] = None | ||
| port: Optional[int] = None | ||
| ssl: Optional[bool] = False | ||
| n_results: Optional[int] = 4 |
There was a problem hiding this comment.
suggest higher default value like 10?
| results = await self.retrieve(text) | ||
| return self.combine_retrieval_results(results) | ||
|
|
||
| async def retrieve_and_generate(self, text: str) -> Any: |
There was a problem hiding this comment.
what does this do and is there a reason it cant be implemented?
| "@aws-sdk/util-dynamodb": "^3.621.0", | ||
| "@libsql/client": "^0.14.0", | ||
| "axios": "^1.7.2", | ||
| "chromadb": "^1.8.1", |
There was a problem hiding this comment.
why did you set 1.8.1 here? the latest version is 2.4.3
| "devDependencies": { | ||
| "@types/jest": "^29.5.12", | ||
| "@types/mocha": "^10.0.7", | ||
| "@types/node": "^20.11.30", |
| port?: number; | ||
| ssl?: boolean; | ||
| nResults?: number; | ||
| embeddingFunction?: any; |
There was a problem hiding this comment.
same as python - should this be required and typed?
|
|
||
| const results = await this.collection.query({ | ||
| queryTexts: [text], | ||
| nResults: this.options.nResults || 4 |
| } | ||
|
|
||
| /** | ||
| * Placeholder for retrieve and generate functionality |
|
|
||
| setup() No newline at end of file | ||
| setup( | ||
| install_requires=[ |
There was a problem hiding this comment.
will the maintainers be ok with this?
|
Hey @ailunc ! Are you still working on this? If not, I can take over and compete this PR |
|
Occupied by other code :) Feel free to take over! |
Issue Link (REQUIRED)
#323
Fixes #323
Summary
Changes
ChromaRetrieverclass andChromaRetrieverOptionsclassChromaRetrieverclass andChromaRetrieverOptionsinterfaceUser experience
Before:
After:
Usage examples:
Python:
TypeScript:
Checklist
Is this a breaking change?
RFC issue number: N/A
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.