[ENH]: adding capability for Spacy embeddings

[Vishnunkumar]

Description of changes

• New functionality: added feature to leverage spacy models

Test plan

• will add unit test cases

Documentation Changes

• Docstrings are added with proper Error Handling.

[github-actions]

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

• Can you think of any use case in which the code does not behave as intended? Have they been tested?
• Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
• If appropriate, are there adequate property based tests?
• If appropriate, are there adequate unit tests?
• Should any logging, debugging, tracing information be added or removed?
• Are error messages user-friendly?
• Have all documentation changes needed been made?
• Have all non-obvious changes been commented?

System Compatibility

• Are there any potential impacts on other parts of the system or backward compatibility?
• Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

• Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

[tazarov]

@Vishnunkumar, this looks good. Thank you.

Can you also add a test for this under chromadb/test/ef/test_spacy_ef.py? We're in the process of refactoring the embedding functions DX with an emphasis on testing and usability.

Did you also create a PR for the docs with links to official docs etc?

[tazarov]

Here are a few points to address:

• Tests
• Docs (unless already done)
• Additional options on the EF:
  • can spacy cache models (if yes, add caching dir with a default value)
  • can other models be loaded other than en_code_web_{model} (if yes, maybe make things more flexible
  • Are there any other constraints that can/should be applied to the library that can/are useful?
  • Is batching supported where multiple docs can be passed in a single round instead of a loop?

[Vishnunkumar]

@Vishnunkumar, this looks good. Thank you.

Can you also add a test for this under chromadb/test/ef/test_spacy_ef.py? We're in the process of refactoring the embedding functions DX with an emphasis on testing and usability. - Working on it with ETA of 2 days

Did you also create a PR for the docs with links to official docs etc? - will look into this

Working on the comments, Thanks for the review @tazarov

[Vishnunkumar]

Sharing the snap of report for test cases.

[Vishnunkumar]

• Tests - Added unit test cases
• Docs PR - working on it: Raised a PR for the same
• spacy models doesn't have any seperate process for caching, please look at this for reference
• can other models be loaded other than en_code_web_{model} (if yes, maybe make things more flexible - Possible models with weights are "md" and "lg", the function loads lg model by default if they are downloaded already. else it will throw an error.
• I believe the below piece of code is handling the multiple docs when the input is list of multiple docs
  cast(Embeddings, [list(self._nlp(doc).vector.astype("float")) for doc in input])

[tazarov]

@Vishnunkumar, looking at spacy docs, I can see that there are many models also, some in other languages - https://spacy.io/models. Also, I see some additional models that trade off accuracy for efficiency. Can we make it so that people can specify those other models as well? e.g., specify the whole model - en_core_web_sm or de_core_news_lg?

[Vishnunkumar]

@Vishnunkumar, looking at spacy docs, I can see that there are many models also, some in other languages - https://spacy.io/models. Also, I see some additional models that trade off accuracy for efficiency. Can we make it so that people can specify those other models as well? e.g., specify the whole model - en_core_web_sm or de_core_news_lg?

• Understood, I was going only for english models, will update on this accordingly.

[Vishnunkumar]

• Tests - Added unit test cases
• Docs PR - working on it: Raised a PR for the same. PR#244
• spacy models doesn't have any seperate process for caching, please look at this for reference
• can other models be loaded other than en_code_web_{model} (if yes, maybe make things more flexible - Possible models with weights are "md" and "lg", the function loads lg model by default if they are downloaded already. else it will throw an error.
• I believe the below piece of code is handling the multiple docs when the input is list of multiple docs
  cast(Embeddings, [list(self._nlp(doc).vector.astype("float")) for doc in input])

Hi @tazarov , I have update with necessary requirments. Please review when you are avaiable

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
chroma	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 13, 2024 4:40am

[tazarov]

This is good. Thanks @Vishnunkumar.

There are only a couple of minor things to look at.

[tazarov]

We can add a portion of this text as match can do loose matching.

[Vishnunkumar]

Resolved

[Vishnunkumar]

This is good. Thanks @Vishnunkumar.

There are only a couple of minor things to look at.

Hi @tazarov , I will look into these.

[Vishnunkumar]

This is good. Thanks @Vishnunkumar.
There are only a couple of minor things to look at.

Hi @tazarov , I will look into these. Please review as per availability

Hi @tazarov, I have updated the changes.

[tazarov]

LGTM. Can I ask you to run the following to make sure everything is formatted as it should:

pip install -r requirements_dev.txt # install dev deps
pre-commit run --all-files trailing-whitespace                                                                                                                                                                                                                                                      
pre-commit run --all-files mixed-line-ending
pre-commit run --all-files end-of-file-fixer
pre-commit run --all-files requirements-txt-fixer
pre-commit run --all-files check-xml
pre-commit run --all-files check-merge-conflict
pre-commit run --all-files check-case-conflict
pre-commit run --all-files check-docstring-first
pre-commit run --all-files black
pre-commit run --all-files flake8
pre-commit run --all-files prettier
pre-commit run --all-files check-yaml

[tazarov]

@Vishnunkumar, thanks for sticking with this. I think this is ready to merge now.

[Vishnunkumar]

@Vishnunkumar, thanks for sticking with this. I think this is ready to merge now.

Hey @tazarov , thanks for reviewing and helping me throughout the PR. Always feeling great to contribute to open source communities.

[Vishnunkumar]

Hey @tazarov, just wanted to check who will be merging this PR? Also it seems the docs repository has been archived. Is there any other repo that I should look for adding docs.

[tazarov]

@Vishnunkumar, we've recently merged the docs repo here alongside a big refactor. The new docs reside in this repo now under https://github.com/chroma-core/chroma/tree/main/docs/docs.trychroma.com/pages/integrations. Just create a new .md, and don't forget to update the sidebar.js