Skip to content
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.

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

feat: support adding and deleting from MySQLVectorStore #53

Merged
merged 2 commits into from
Mar 27, 2024

Conversation

jackwotherspoon
Copy link
Contributor

Add support for .add_documents and .delete MySQLVector methods.

@jackwotherspoon jackwotherspoon self-assigned this Mar 26, 2024
@jackwotherspoon jackwotherspoon requested a review from a team as a code owner March 26, 2024 22:30
@product-auto-label product-auto-label bot added the api: cloudsql-mysql Issues related to the googleapis/langchain-google-cloud-sql-mysql-python API. label Mar 26, 2024
self,
ids: Optional[List[str]] = None,
**kwargs: Any,
) -> Optional[bool]:
Copy link
Contributor Author

@jackwotherspoon jackwotherspoon Mar 26, 2024

Choose a reason for hiding this comment

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

not sure why this return type is optional Optional[bool] but that is what Postgres had... 🤷‍♂️

Copy link
Collaborator

Choose a reason for hiding this comment

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

This matches the VS interface (https://github.com/langchain-ai/langchain/blob/master/libs/core/langchain_core/vectorstores.py#L88). Should we change it to just bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the link! I think the VS interface is basically just saying subclasses can choose to return True or return None for successfully deleted items. Leaving it up to the individual implementations to decide which approach they prefer. Since we are returning True/False with no path that returns None I've updated the return type to be -> bool to make this clear to users.

self,
ids: Optional[List[str]] = None,
**kwargs: Any,
) -> Optional[bool]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This matches the VS interface (https://github.com/langchain-ai/langchain/blob/master/libs/core/langchain_core/vectorstores.py#L88). Should we change it to just bool?

@jackwotherspoon jackwotherspoon merged commit ce45617 into main Mar 27, 2024
10 checks passed
@jackwotherspoon jackwotherspoon deleted the add-and-delete branch March 27, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: cloudsql-mysql Issues related to the googleapis/langchain-google-cloud-sql-mysql-python API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants