Harrison/matching engine vectorstore #3350
Conversation
…ments in from_texts to make them required.
|
@tomaspiaggio i tried to update this and get it to pass lint - how does it look? |
There was a problem hiding this comment.
@hwchase17 some minor details. Otherwise, it lgtm. lmk if you need anything else from me!
| **kwargs: Any, | ||
| ) -> "MatchingEngine": | ||
| """Use from components instead.""" | ||
| raise NotImplementedError( |
There was a problem hiding this comment.
@hwchase17 why was this removed? Previously, it would have worked regardless if the developer called from_texts or from_components
There was a problem hiding this comment.
think the from_texts interface doesn't fit super well, since it only allows texts and embedding as args, whereas the init and from_components both expect a number of args. we could technically pipe all kwargs for from_text into one of those two but it's pretty hacky. also there's no real value to using from_texts for this class, since it'd just be wrapping instantiation and calling add_texts
|
@hwchase17 really interested in seeing this get merged! Open to me fixing tomaspiaggio's comments? |
docs/modules/indexes/vectorstores/examples/matchingengine.ipynb
Outdated
Show resolved
Hide resolved
Co-authored-by: Tom Piaggio <tomaspiaggio@google.com> Co-authored-by: scafati98 <jupyter@matchingengine.us-central1-a.c.scafati-joonix.internal> Co-authored-by: scafati98 <scafatieugenio@gmail.com> Co-authored-by: Dev 2049 <dev.dev2049@gmail.com>
Co-authored-by: Tom Piaggio <tomaspiaggio@google.com> Co-authored-by: scafati98 <jupyter@matchingengine.us-central1-a.c.scafati-joonix.internal> Co-authored-by: scafati98 <scafatieugenio@gmail.com> Co-authored-by: Dev 2049 <dev.dev2049@gmail.com>
No description provided.