Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function #6876

0xcha05 · 2023-06-28T15:41:28Z

Description:

Updated the delete function in the Pinecone integration to allow for deletion of vectors by specifying a filter condition, and to delete all vectors in a namespace.

Made the ids parameter optional in the delete function in the base VectorStore class and allowed for additional keyword arguments.

Updated the delete function in several classes (Redis, Chroma, Supabase, Deeplake, Elastic, Weaviate, and Cassandra) to match the changes made in the base VectorStore class. This involved making the ids parameter optional and allowing for additional keyword arguments.

vercel · 2023-06-28T15:41:31Z

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

Name	Status	Preview	Comments	Updated (UTC)
langchain	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 2, 2023 4:29pm

CodiumAI-Agent

PR Analysis

🎯 Main theme: The PR enhances the delete function in the Pinecone integration by adding the ability to delete vectors by specifying a filter condition or delete all vectors in a namespace.
🔍 Description and title: Yes
📌 Type of PR: Enhancement
🧪 Relevant tests added: No
⚠️ Unrelated changes: No
✨ Minimal and focused: Yes, this PR is small and focuses only on enhancing the delete function in the Pinecone integration.

PR Feedback

💡 Suggestions: The PR is well-structured and the changes are clear. However, it would be beneficial to include tests that demonstrate the new functionality. This would help ensure that the changes work as expected and prevent potential regressions in the future.
🌱 Minor suggestions: Consider adding more detailed docstrings for the delete function. This would help users understand the different ways they can use the function and what they should expect as a result.
🤖 Code suggestions:
- In the delete function, consider handling the case where both ids and filter are provided. Currently, the function only deletes by ids if they are provided, even if a filter is also provided. This could lead to unexpected behavior for users. [important]
- In the delete function, consider returning a meaningful message or result when no vectors are deleted because none match the provided ids or filter. This would provide users with more information about the result of their delete operation. [medium]

Comment 'CodiumAI please review' to ask for another review after you update the PR.

coditamar · 2023-06-28T21:07:52Z

langchain/vectorstores/pinecone.py

+            chunk_size = 1000
+            for i in range(0, len(ids), chunk_size):
+                chunk = ids[i : i + chunk_size]
+                return self._index.delete(ids=chunk, namespace=namespace, **kwargs)


Doesn't this mean that the loop won't actually run over all chunks and break on the first one?

that's true, great catch. Thanks!

I actually ran CodiumAI-Agent a few times on your code (sorry, mistakenly, two times was published here, apologize),
and in one of these cases, it suggested this error 🤖

I love the idea and execution, it's very helpful. I'd love to contribute if anything's open source!

@coditamar this is great. how can I run on PR's regularly?

@rlancemartin , we will try to release an option to run regularly this coming week!

@rlancemartin , here you go:
https://github.com/Codium-ai/pr-agent
would love to discuss how to use 'LangChain' in 'pr-agent'

dev2049 · 2023-06-29T01:37:58Z

langchain/vectorstores/pinecone.py

-    def delete(self, ids: List[str], namespace: Optional[str] = None) -> None:
-        """Delete by vector IDs.
+
+    def delete(self,


seems like delete is implemented on base VectorStore class as delete(self, ids: List[str]), so we shouldn't change signature (which I recognize Pinecone implementation already did, but should fix that). i suggest we either:

update VectorStore.delete to have signature more like delete(self, ids: Optional[List[str]]=None, **kwargs: Any) or rename it to delete_by_ids(self, ids: List[str]) and then we can keep this implementation as is

rename this method to something like delete_with_kwargs (maybe there's a better name)

think i prefer (1) but curious to hear what others think, cc @rlancemartin @eyurtsev

agreed. I think 1 is best also.

good point. @0xcha05 how do you recommend modifying the base VectorStore class delete method? IMO delete(self, ids: Optional[List[str]]=None, **kwargs: Any) is reasonable.

the central point is that there are various uses of delete so do not require IDs, so base VectorStore should be modified. do you want to take that up in this PR?

yes, i'll change the vecstorestore class delete method to delete(self, ids: Optional[List[str]]=None, **kwargs: Any) , will do this in this PR.

rlancemartin · 2023-06-29T22:12:25Z

Nice addition! As mentioned by @dev2049 we should updated the base VectorStore delete class here and above suggestion is reasonable to me.

delete(self, ids: Optional[List[str]]=None, **kwargs: Any)

A number of vectorstores have delete methods w/ ids passed, and this would of course be backwards compatible. But making IDs optional is more flexible for future additions, like the nice one you propose here.

…optional IDs and additional keyword arguments

0xcha05 · 2023-06-30T04:25:50Z

I've refactored delete method in VectorStore and Pinecone classes to accept optional IDs and additional keyword arguments.
CC: @rlancemartin @dev2049 @eyurtsev

rlancemartin

Discussing w/ @nfcampos and @jacoblee93 who are adding this same functionality to js - there is possible confusion if a user passes both filter params and IDs. In this case, ID-wise delete is done first. Thoughts on having independent methods delete_by_id and delete_by_filter? Pinecone simply has delete as done here, so perhaps separate methods also has drawbacks. https://docs.pinecone.io/reference/delete_post

jacoblee93 · 2023-07-01T14:52:02Z

Discussing w/ @nfcampos and @jacoblee93 who are adding this same functionality to js - there is possible confusion if a user passes both filter params and IDs. In this case, ID-wise delete is done first. Thoughts on having independent methods delete_by_id and delete_by_filter? Pinecone simply has delete as done here, so perhaps separate methods also has drawbacks. https://docs.pinecone.io/reference/delete_post

There could also be additional one-off options for other vector stores in the future - if we do separate methods we can always refactor to a single combined one if there's confusion.

0xcha05 · 2023-07-01T15:04:19Z

the pinecone doc says that "If specified, the metadata filter here will be used to select the vectors to delete. This is mutually exclusive
with specifying IDs to delete in the ids param or using delete_all=True."
so we can just have the pinecone delete function with ids optional and kwargs, and have some validation in delete function of the pinecone class. what do you say? @rlancemartin

jacoblee93 · 2023-07-01T15:10:25Z

the pinecone doc says that "If specified, the metadata filter here will be used to select the vectors to delete. This is mutually exclusive
with specifying IDs to delete in the ids param or using delete_all=True."
so we can just have the pinecone delete function with ids optional and kwargs, and have some validation in delete function of the pinecone class. what do you say?

I think it sounds fine for Pinecone and what devs are used to there, just not sure it's going to be so clean and clear as an interface extended to other vector stores since they might have different behavior/expected precedence for args like this and we're adding this to the base class.

0xcha05 · 2023-07-01T15:28:10Z

Absolutely, let's clarify things a bit. The base delete function is only being tweaked to make ids optional and to allow for additional keyword arguments, like so:
def delete(self, ids: Optional[List[str]] = None, **kwargs: Any) -> Optional[bool]:
Here's the commit for reference.

So, this change shouldn't disrupt any existing code that uses the delete method. The filter and delete_all parameters are specific to the Pinecone subclass and won't affect the base class or any other subclasses. Please correct me if I'm missing something!

rlancemartin · 2023-07-01T17:25:26Z

Absolutely, let's clarify things a bit. The base delete function is only being tweaked to make ids optional and to allow for additional keyword arguments, like so: def delete(self, ids: Optional[List[str]] = None, **kwargs: Any) -> Optional[bool]: Here's the commit for reference.

So, this change shouldn't disrupt any existing code that uses the delete method. The filter and delete_all parameters are specific to the Pinecone subclass and won't affect the base class or any other subclasses. Please correct me if I'm missing something!

+1, imo this is reasonable.

rlancemartin

With update to base vectorstore, we need to make ids: Optional [List[str]] in the delete method for:

1/ redis
2/ chroma
3/ supabase
4/ deeplake
5/ elastic
6/ weaviate

This will resolve Lint errors.

0xcha05 · 2023-07-01T18:08:40Z

I'll do this in this PR, @rlancemartin .

…nd Weaviate classes to make ids optional and accept additional kwargs

0xcha05 · 2023-07-01T18:40:41Z

The reason i did

filter = kwargs.get('filter')
delete_all = kwargs.get('delete_all')

in deeplake is because I didn't want to change the current behaviour. but passing kwargs directly into self.vectorstore.delete would be good.

0xcha05 · 2023-07-01T19:47:44Z

I've fixed the lint issues, but not sure what the Cassandra lint issue is about. There isn't a delete function implemented. @rlancemartin

rlancemartin · 2023-07-01T22:41:45Z

I've fixed the lint issues, but not sure what the Cassandra lint issue is about. There isn't a delete function implemented. @rlancemartin

Will have a look, just kicked off tests.

rlancemartin · 2023-07-02T14:38:43Z

I've fixed the lint issues, but not sure what the Cassandra lint issue is about. There isn't a delete function implemented. @rlancemartin

Will have a look, just kicked off tests.

@0xcha05 Cassandra also has delete -- can you please change signature as done for the other DBs. That should resolve Lint and we can get this in. (See delete() in cassandra.py.)

0xcha05 · 2023-07-02T14:50:41Z

Thank you, the version I had didn't have a delete method in Cassandra. but this should work now, please let me know if this looks good @rlancemartin .

0xcha05 · 2023-07-02T16:26:35Z

I've reformatted with black. This should fix all lint errors.

* master: (212 commits) Add SpacyEmbeddings class (langchain-ai#6967) docs: commented out `editUrl` option (langchain-ai#6440) Remove duplicate mongodb integration doc (langchain-ai#7006) Update get_started.mdx (langchain-ai#7005) openapi chain nit (langchain-ai#7012) Fix sample in FAISS section (langchain-ai#7050) Fix typo in google_places_api.py (langchain-ai#7055) move base prompt to schema (langchain-ai#6995) added `Brave Search` document_loader (langchain-ai#6989) Add JSON Lines support to JSONLoader (langchain-ai#6913) Vectara upd2 (langchain-ai#6506) docstrings `document_loaders` 2 (langchain-ai#6890) docstrings `document_loaders` 1 (langchain-ai#6847) Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function (langchain-ai#6876) bump 221 (langchain-ai#7047) Rm retriever kwargs (langchain-ai#7013) Polish reference docs (langchain-ai#7045) Support params on GoogleSearchApiWrapper (langchain-ai#6810) (langchain-ai#7014) Fix typo (langchain-ai#7023) Fix openai multi functions agent docs (langchain-ai#7028) ...

…egration, updated base VectorStore's delete function (#6876) ### Description: Updated the delete function in the Pinecone integration to allow for deletion of vectors by specifying a filter condition, and to delete all vectors in a namespace. Made the ids parameter optional in the delete function in the base VectorStore class and allowed for additional keyword arguments. Updated the delete function in several classes (Redis, Chroma, Supabase, Deeplake, Elastic, Weaviate, and Cassandra) to match the changes made in the base VectorStore class. This involved making the ids parameter optional and allowing for additional keyword arguments.

…egration, updated base VectorStore's delete function (langchain-ai#6876) ### Description: Updated the delete function in the Pinecone integration to allow for deletion of vectors by specifying a filter condition, and to delete all vectors in a namespace. Made the ids parameter optional in the delete function in the base VectorStore class and allowed for additional keyword arguments. Updated the delete function in several classes (Redis, Chroma, Supabase, Deeplake, Elastic, Weaviate, and Cassandra) to match the changes made in the base VectorStore class. This involved making the ids parameter optional and allowing for additional keyword arguments.

Added filter option to delete function in Pinecone integration

370d12d

dosubot bot added the 03 enhancement Enhancement of existing functionality label Jun 28, 2023

vercel bot temporarily deployed to Preview June 28, 2023 15:47 Inactive

added delete all

63555e8

0xcha05 changed the title ~~Added filter option to delete function in Pinecone integration~~ Added filter and delete all option to delete function in Pinecone integration Jun 28, 2023

vercel bot temporarily deployed to Preview June 28, 2023 16:08 Inactive

CodiumAI-Agent reviewed Jun 28, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

coditamar reviewed Jun 28, 2023

View reviewed changes

Fixed bug in delete function by removing premature return statement

47ffabf

vercel bot temporarily deployed to Preview June 28, 2023 21:25 Inactive

dev2049 removed the 03 enhancement Enhancement of existing functionality label Jun 29, 2023

dev2049 assigned rlancemartin Jun 29, 2023

dev2049 reviewed Jun 29, 2023

View reviewed changes

Refactor delete method in VectorStore and Pinecone classes to accept …

964d720

…optional IDs and additional keyword arguments

vercel bot temporarily deployed to Preview June 30, 2023 04:31 Inactive

0xcha05 requested a review from rlancemartin July 1, 2023 03:04

rlancemartin reviewed Jul 1, 2023

View reviewed changes

Update delete method in Redis, Chroma, Supabase, Deeplake, Elastic, a…

bda2fc3

…nd Weaviate classes to make ids optional and accept additional kwargs

0xcha05 requested a review from rlancemartin July 1, 2023 18:28

vercel bot temporarily deployed to Preview July 1, 2023 18:34 Inactive

fix lint

a2df55f

vercel bot temporarily deployed to Preview July 1, 2023 19:47 Inactive

0xcha05 added 2 commits July 2, 2023 20:16

cassandra delete function link fix

8d47c66

Merge branch 'master' into master

e3a04cc

vercel bot temporarily deployed to Preview July 2, 2023 15:00 Inactive

0xcha05 changed the title ~~Added filter and delete all option to delete function in Pinecone integration~~ Added filter and delete all option to delete function in Pinecone integration, change the base vectorstore class's delete function Jul 2, 2023

0xcha05 changed the title ~~Added filter and delete all option to delete function in Pinecone integration, change the base vectorstore class's delete function~~ Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function Jul 2, 2023

black format

942f1cb

vercel bot temporarily deployed to Preview July 2, 2023 16:29 Inactive

rlancemartin merged commit e41b382 into langchain-ai:master Jul 2, 2023
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function #6876

Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function #6876

0xcha05 commented Jun 28, 2023 •

edited

vercel bot commented Jun 28, 2023 •

edited

CodiumAI-Agent left a comment

This comment was marked as duplicate.

coditamar Jun 28, 2023

0xcha05 Jun 28, 2023

coditamar Jun 28, 2023

0xcha05 Jun 28, 2023

rlancemartin Jul 1, 2023

coditamar Jul 2, 2023

coditamar Jul 6, 2023

dev2049 Jun 29, 2023

0xcha05 Jun 29, 2023

rlancemartin Jun 29, 2023 •

edited

0xcha05 Jun 30, 2023

rlancemartin commented Jun 29, 2023 •

edited

0xcha05 commented Jun 30, 2023

rlancemartin left a comment •

edited

jacoblee93 commented Jul 1, 2023 •

edited

0xcha05 commented Jul 1, 2023 •

edited

jacoblee93 commented Jul 1, 2023 •

edited

0xcha05 commented Jul 1, 2023

rlancemartin commented Jul 1, 2023

rlancemartin left a comment

0xcha05 commented Jul 1, 2023

0xcha05 commented Jul 1, 2023

0xcha05 commented Jul 1, 2023 •

edited

rlancemartin commented Jul 1, 2023

rlancemartin commented Jul 2, 2023

0xcha05 commented Jul 2, 2023

0xcha05 commented Jul 2, 2023

Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function #6876

Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function #6876

Conversation

0xcha05 commented Jun 28, 2023 • edited

Description:

vercel bot commented Jun 28, 2023 • edited

CodiumAI-Agent left a comment

Choose a reason for hiding this comment

PR Analysis

PR Feedback

This comment was marked as duplicate.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlancemartin Jun 29, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlancemartin commented Jun 29, 2023 • edited

0xcha05 commented Jun 30, 2023

rlancemartin left a comment • edited

Choose a reason for hiding this comment

jacoblee93 commented Jul 1, 2023 • edited

0xcha05 commented Jul 1, 2023 • edited

jacoblee93 commented Jul 1, 2023 • edited

0xcha05 commented Jul 1, 2023

rlancemartin commented Jul 1, 2023

rlancemartin left a comment

Choose a reason for hiding this comment

0xcha05 commented Jul 1, 2023

0xcha05 commented Jul 1, 2023

0xcha05 commented Jul 1, 2023 • edited

rlancemartin commented Jul 1, 2023

rlancemartin commented Jul 2, 2023

0xcha05 commented Jul 2, 2023

0xcha05 commented Jul 2, 2023

0xcha05 commented Jun 28, 2023 •

edited

vercel bot commented Jun 28, 2023 •

edited

rlancemartin Jun 29, 2023 •

edited

rlancemartin commented Jun 29, 2023 •

edited

rlancemartin left a comment •

edited

jacoblee93 commented Jul 1, 2023 •

edited

0xcha05 commented Jul 1, 2023 •

edited

jacoblee93 commented Jul 1, 2023 •

edited

0xcha05 commented Jul 1, 2023 •

edited