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

Fix update_document function, add test and documentation. #5359

Merged

Conversation

martinholecekmax
Copy link
Contributor

Fix for update_document Function in Chroma

Summary

This pull request addresses an issue with the update_document function in the Chroma class, as described in #5031. The issue was identified as an AttributeError raised when calling update_document due to a missing corresponding method in the Collection object. This fix refactors the update_document method in Chroma to correctly interact with the Collection object.

Changes

  1. Fixed the update_document method in the Chroma class to correctly call methods on the Collection object.
  2. Added the corresponding test test_chroma_update_document in tests/integration_tests/vectorstores/test_chroma.py to reflect the updated method call.
  3. Added an example and explanation of how to use the update_document function in the Jupyter notebook tutorial for Chroma.

Test Plan

All existing tests pass after this change. In addition, the test_chroma_update_document test case now correctly checks the functionality of update_document, ensuring that the function works as expected and updates the content of documents correctly.

Reviewers

@dev2049

This fix will ensure that users are able to use the update_document function as expected, without encountering the previous AttributeError. This will enhance the usability and reliability of the Chroma class for all users.

Thank you for considering this pull request. I look forward to your feedback and suggestions.

@hwchase17
Copy link
Contributor

@atroyn @jeffchuber does this look okay?

@jeffchuber
Copy link
Contributor

@hwchase17 im working on a larger cleanup / refactor of the chroma integration. We can land this and I will probably make a few tweaks in my PR. As far as I know update_document has never been in the Chroma API 🤷 , which I had noticed. Glad this got cleaned up! thanks @martinholecekmax

@martinholecekmax
Copy link
Contributor Author

Hi @jeffchuber,

I'm glad to hear that you're undertaking a larger cleanup and refactoring of the Chroma integration. That sounds like a fantastic initiative! I look forward to seeing the improvements and tweaks you'll be making.

Yes, it was indeed puzzling to discover that update_document was not in the Chroma API as expected. I'm glad we could address this issue and improve the functionality of the Chroma class.

I appreciate your feedback and support on this. Let me know if there's anything specific you'd like me to be aware of or contribute to in your ongoing refactoring efforts.

Thanks

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.

@hwchase17 hwchase17 merged commit 44b48d9 into langchain-ai:master May 29, 2023
@martinholecekmax martinholecekmax deleted the chroma_update_document branch May 29, 2023 14:32
vowelparrot pushed a commit that referenced this pull request May 31, 2023
# Fix for `update_document` Function in Chroma

## Summary
This pull request addresses an issue with the `update_document` function
in the Chroma class, as described in
[#5031](#5031 (comment)).
The issue was identified as an `AttributeError` raised when calling
`update_document` due to a missing corresponding method in the
`Collection` object. This fix refactors the `update_document` method in
`Chroma` to correctly interact with the `Collection` object.

## Changes
1. Fixed the `update_document` method in the `Chroma` class to correctly
call methods on the `Collection` object.
2. Added the corresponding test `test_chroma_update_document` in
`tests/integration_tests/vectorstores/test_chroma.py` to reflect the
updated method call.
3. Added an example and explanation of how to use the `update_document`
function in the Jupyter notebook tutorial for Chroma.

## Test Plan
All existing tests pass after this change. In addition, the
`test_chroma_update_document` test case now correctly checks the
functionality of `update_document`, ensuring that the function works as
expected and updates the content of documents correctly.

## Reviewers
@dev2049

This fix will ensure that users are able to use the `update_document`
function as expected, without encountering the previous
`AttributeError`. This will enhance the usability and reliability of the
Chroma class for all users.

Thank you for considering this pull request. I look forward to your
feedback and suggestions.
@danielchalef danielchalef mentioned this pull request Jun 5, 2023
Undertone0809 pushed a commit to Undertone0809/langchain that referenced this pull request Jun 19, 2023
…ai#5359)

# Fix for `update_document` Function in Chroma

## Summary
This pull request addresses an issue with the `update_document` function
in the Chroma class, as described in
[langchain-ai#5031](langchain-ai#5031 (comment)).
The issue was identified as an `AttributeError` raised when calling
`update_document` due to a missing corresponding method in the
`Collection` object. This fix refactors the `update_document` method in
`Chroma` to correctly interact with the `Collection` object.

## Changes
1. Fixed the `update_document` method in the `Chroma` class to correctly
call methods on the `Collection` object.
2. Added the corresponding test `test_chroma_update_document` in
`tests/integration_tests/vectorstores/test_chroma.py` to reflect the
updated method call.
3. Added an example and explanation of how to use the `update_document`
function in the Jupyter notebook tutorial for Chroma.

## Test Plan
All existing tests pass after this change. In addition, the
`test_chroma_update_document` test case now correctly checks the
functionality of `update_document`, ensuring that the function works as
expected and updates the content of documents correctly.

## Reviewers
@dev2049

This fix will ensure that users are able to use the `update_document`
function as expected, without encountering the previous
`AttributeError`. This will enhance the usability and reliability of the
Chroma class for all users.

Thank you for considering this pull request. I look forward to your
feedback and suggestions.
This was referenced Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants