Skip to content

Adding Marqo to vectorstore ecosystem #7068

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.

Sign up for GitHub

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

Jump to bottom
Merged
rlancemartin merged 15 commits into from
Jul 5, 2023
Merged

Adding Marqo to vectorstore ecosystem #7068

rlancemartin merged 15 commits into from
Jul 5, 2023

Conversation

@tomhamer
Copy link
Contributor

@tomhamer tomhamer commented Jul 3, 2023

This PR brings in a vectorstore interface for Marqo.

The Marqo vectorstore exposes some of Marqo's functionality in addition the the VectorStore base class. The Marqo vectorstore also makes the embedding parameter optional because inference for embeddings is an inherent part of Marqo.

Docs, notebook examples and integration tests included.

Related PR:
#2807

@vercel
Copy link

vercel bot commented Jul 3, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jul 5, 2023 9:30pm

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 3, 2023

For now, I've moved this across to a new PR as the harrison/marqo branch needs substantial merging and can't automatically merged across. Do you prefer me to create a new PR to harrison/marqo?

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 4, 2023

@baskaryan @rlancemartin everything looks to be passing now. Let me know if there is anything more to be done on this, happy to do whatever is needed to get this across the line. Thanks so much!

@rlancemartin
Copy link
Contributor

rlancemartin commented Jul 4, 2023

@baskaryan @rlancemartin everything looks to be passing now. Let me know if there is anything more to be done on this, happy to do whatever is needed to get this across the line. Thanks so much!

Great. Just kicked off tests and will merge once we confirm all pass.

@rlancemartin
Copy link
Contributor

rlancemartin commented Jul 5, 2023

@baskaryan @rlancemartin everything looks to be passing now. Let me know if there is anything more to be done on this, happy to do whatever is needed to get this across the line. Thanks so much!

Great. Just kicked off tests and will merge once we confirm all pass.

Please run "make format" to resolve Lint errors.

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

@rlancemartin sorry about that - completed now

@rlancemartin
Copy link
Contributor

rlancemartin commented Jul 5, 2023

@rlancemartin sorry about that - completed now

np! looks like an issue w/ poetry.lock?

Error: poetry.lock is not consistent with pyproject.toml. Run `poetry lock [--no-update]` to fix it.

try this:

git checkout master poetry.lock
poetry lock --no-update

gets the latest poetry.lock and updates it w/ your pyproject.toml.

hwchase17
hwchase17 approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@hwchase17 hwchase17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! thanks

@hwchase17 hwchase17 added the lgtm label Jul 5, 2023
@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

Thanks for the help @hwchase17! I was just getting that fixed up but you're right ahead of me. Super impressive how helpful you've been.

@rlancemartin
Copy link
Contributor

rlancemartin commented Jul 5, 2023

Thanks for the help @hwchase17! I was just getting that fixed up but you're right ahead of me. Super impressive how helpful you've been.

A number of Lint errors due to line length. One of us can take this if you don't have time to do it soon. We'll plan to get this merged today.

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

working on it now

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

poetry run ruff . now passing. @rlancemartin did you want to try triggering the workflow again? really appreciate it

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

sorry one moment, ive accidentally added some unneeded lock file changes

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

Done, should be good to go

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

Looks like I didnt lint the file in the correct way, going to try again with Black

@tomhamer
Copy link
Contributor Author

tomhamer commented Jul 5, 2023

@rlancemartin thanks so much for your help! Looks like everything passed 🎉

@rlancemartin rlancemartin merged commit e533da8 into langchain-ai:master Jul 5, 2023
@OwenElliottDev OwenElliottDev mentioned this pull request Jul 7, 2023
Merged
baskaryan pushed a commit that referenced this pull request Jul 7, 2023
@OwenElliottDev
Marqo Vector Store Examples & Type Hints (#7326)
3074306 
This PR improves the example notebook for the Marqo vectorstore
implementation by adding a new RetrievalQAWithSourcesChain example. The
`embedding` parameter in `from_documents` has its type updated to
`Union[Embeddings, None]` and a default parameter of None because this
is ignored in Marqo.

This PR also upgrades the Marqo version to 0.11.0 to remove the device
parameter after a breaking change to the API.

Related to #7068 @tomhamer @hwchase17

---------

Co-authored-by: Tom Hamer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hwchase17 hwchase17 hwchase17 approved these changes

Assignees

@rlancemartin rlancemartin

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tomhamer @rlancemartin @hwchase17 @baskaryan