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

much improved doc strings for client #290

Merged
merged 2 commits into from
Apr 22, 2023
Merged

much improved doc strings for client #290

merged 2 commits into from
Apr 22, 2023

Conversation

jeffchuber
Copy link
Contributor

Improved docstrings for chroma-core/docs#28

@jeffchuber jeffchuber requested a review from HammadB April 4, 2023 21:17
The newly created collection

Raises:
ValueError: If the collection already exists and get_or_create is False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't there also some error if the collection doesn't follow naming guidelines?

The collection

Examples:
>>> client.get_or_create_collection("my_collection")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is awesome.

None

Raises:
ValueError: If you don't provide either embeddings or documents
Copy link
Collaborator

@HammadB HammadB Apr 4, 2023

Choose a reason for hiding this comment

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

There are a bunch of validation errors that could also be thrown? Do we want to surface those here?

GetResult: A GetResult object containing the results.

Raises:
ValueError: If you provide both ids and a where filter
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really do this? I think we support both.

Copy link
Collaborator

@HammadB HammadB left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. It looks good minus the validaty of the errors I wasn't sure about. Also, can we decide if we want to surface other errors? I'm pretty sure there are errors in validation that we aren't surfacing in the docstrings.

@jeffchuber jeffchuber merged commit 79c891f into main Apr 22, 2023
@jeffchuber jeffchuber deleted the pydoc branch April 22, 2023 01:21
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.

2 participants