VectorStore bug fix #2420
VectorStore bug fix #2420
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2420 +/- ##
==========================================
- Coverage 84.69% 84.67% -0.03%
==========================================
Files 326 326
Lines 37552 37548 -4
==========================================
- Hits 31806 31795 -11
- Misses 5746 5753 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| if kwargs["embedding_data"] is None and kwargs["embedding_function"] is not None: | ||
| raise ValueError( | ||
| f"When an `embedding_function` is specified, `embedding_data` must also be specified." | ||
| ) | ||
|
|
||
| if ( |
There was a problem hiding this comment.
is this logic correct? The error does not seem to match the check.
There was a problem hiding this comment.
yup, error message should say a vice versa
| @@ -99,11 +100,26 @@ def parse_search_args(**kwargs): | |||
| raise ValueError( | |||
| f"Either an `embedding`, `embedding_function`, `filter`, or `query` must be specified." | |||
| ) | |||
|
|
|||
| if kwargs["embedding"] is not None and kwargs["embedding_function"]: | |||
There was a problem hiding this comment.
Should it be kwargs["embedding_data"] here?
There was a problem hiding this comment.
I see. The idea was that embedding is the embedding data, but since now we have embedding_data which serves a bit different purpose, this error message is confusing.
|
|
||
| utils.parse_search_args( | ||
| embedding_data=embedding_data, | ||
| embedding_function=embedding_function, | ||
| initial_embedding_function=self.embedding_function, |
There was a problem hiding this comment.
is this used to change the embedding function?
There was a problem hiding this comment.
the idea is that embedding function can be specified from the very beginning, which is called initial_embedding_function
🚀 🚀 Pull Request
Checklist:
coverage-rateupChanges