Skip to content

Conversation

@gRedHeadphone
Copy link
Contributor

original PR: #231

On top of it adding limit and offset parameter to get methods similar to chroma vector store

Copy link
Collaborator

@dishaprakash dishaprakash left a comment

Choose a reason for hiding this comment

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

Can we also add some examples related to this new feature into the vector store how to notebook?

@gRedHeadphone
Copy link
Contributor Author

@dishaprakash done

Copy link
Collaborator

@dishaprakash dishaprakash left a comment

Choose a reason for hiding this comment

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

LGTM

@dishaprakash
Copy link
Collaborator

@gRedHeadphone as we add new features to PGVectorStore, lets also update the how to guide for PGVectorstore on the langchain website (ref PR - langchain-ai/langchain#32549)

results = result_map.fetchall()
return bool(len(results) == 1)

async def aget(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also support an ids input and just call the get_by_ids method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but then other parameters won't work. How about updating filters with ids filter?


async def aget(
self,
filter: Optional[dict] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We most likely should use "where" to match Chroma

limit: Optional[int] = None,
offset: Optional[int] = None,
**kwargs: Any,
) -> list[Document]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This method should return A dict with the keys "ids", "embeddings", "metadatas", "documents" to match the Chroma implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so should I also add "include" parameter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants