-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[ENH] Update JS client for auto ID generation when add #2757
[ENH] Update JS client for auto ID generation when add #2757
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @spikechroma and the rest of your teammates on Graphite |
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
Please tag your PR title with one of: [ENH | BUG | DOC | TST | BLD | PERF | TYP | CLN | CHORE]. See https://docs.trychroma.com/contributing#contributing-code-and-ideas |
8999e52
to
0bd22d1
Compare
f7e0a52
to
9dd5ac7
Compare
8b6579a
to
e918efe
Compare
5ee28b8
to
6a394a1
Compare
223c5e3
to
3b7f57c
Compare
887cdd3
to
f0b6948
Compare
3b7f57c
to
0e0243e
Compare
f0b6948
to
574246a
Compare
0e0243e
to
9a606f2
Compare
574246a
to
568dfd5
Compare
9a606f2
to
b36d182
Compare
568dfd5
to
2ae1a7e
Compare
7df8bc4
to
258bc1b
Compare
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
const recordSet = arrayifyParams(reqParams); | ||
const { ids, embeddings, documents } = recordSet; | ||
|
||
if (!ids) { | ||
throw new Error("ids cannot be undefined"); | ||
} | ||
|
||
validateIDs(ids); | ||
|
||
if (documents) { | ||
recordSet.embeddings = await computeEmbeddings( | ||
embeddingFunction, | ||
embeddings, | ||
documents, | ||
); | ||
} | ||
|
||
return recordSet as MultiRecordOperationParams; |
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.
const recordSet = arrayifyParams(reqParams); | |
const { ids, embeddings, documents } = recordSet; | |
if (!ids) { | |
throw new Error("ids cannot be undefined"); | |
} | |
validateIDs(ids); | |
if (documents) { | |
recordSet.embeddings = await computeEmbeddings( | |
embeddingFunction, | |
embeddings, | |
documents, | |
); | |
} | |
return recordSet as MultiRecordOperationParams; | |
const recordSet: MultiRecordOperationParams = arrayifyParams(reqParams); | |
const { ids, embeddings, documents } = recordSet; | |
if (!ids) { | |
throw new Error("ids cannot be undefined"); | |
} | |
validateIDs(ids); | |
if (documents) { | |
recordSet.embeddings = await computeEmbeddings( | |
embeddingFunction, | |
embeddings, | |
documents, | |
); | |
} | |
return recordSet; |
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.
arrayifyParams
actually gives MultiRecordOperationParamsWithIDsOptional
type instead of MultiRecordOperationParams
``` | ||
npm install | ||
npm run build | ||
``` | ||
|
||
### Publishing | ||
|
||
First build the package then run `npm publish` | ||
First build the package then run ```npm publish``` |
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.
should this be single backticks?
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.
This is auto generated
a70ae76
to
8fc14a5
Compare
007039e
to
1a72e6b
Compare
ccc6653
to
bc71c6c
Compare
1a72e6b
to
36b17a0
Compare
0991ccf
to
6e1ff74
Compare
36b17a0
to
87ba120
Compare
6e1ff74
to
805ea9e
Compare
87ba120
to
499a81c
Compare
7a9987e
to
b338b1e
Compare
b338b1e
to
8473aa0
Compare
8473aa0
to
d676e9b
Compare
499a81c
to
a3dc87d
Compare
d676e9b
to
b726474
Compare
a3dc87d
to
77aeac9
Compare
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?