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

Refactor backend -> model #176

Merged
merged 49 commits into from Jun 29, 2023
Merged

Refactor backend -> model #176

merged 49 commits into from Jun 29, 2023

Conversation

rmitsch
Copy link
Collaborator

@rmitsch rmitsch commented Jun 19, 2023

Description

Refactor to switch from a backend to a model paradigm.
Todos:

  • Readme updates
  • Update Streamlit demo

Types of change

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran all tests in tests and usage_examples/tests, and all new and existing tests passed. This includes
    • all external tests (i. e. pytest ran with --external)
    • all tests requiring a GPU
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@rmitsch rmitsch added the refactoring Refactoring of existing code label Jun 19, 2023
@rmitsch rmitsch self-assigned this Jun 19, 2023
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@rmitsch
Copy link
Collaborator Author

rmitsch commented Jun 28, 2023

Mostly looks good to me! Let's start wrapping this up? Raphael can you update the readme & double check the usage examples, and maybe write a short "migration" guide we can link to?

Added migration guide and one last adjustment (name has to be specified explicitly for LangChain models now outside of the config dict to be more consistent with other models) in 502697f.

Running one last round of tests currently.

@svlandeg
Copy link
Member

We can't try to catch those specific errors from confection, to provide a more specific/helpful warning in spacy-llm for those specific cases? Like if there's an llm.backend config section - couldn't we even check that up-front in some cases?

I wouldn't know where to. Maybe I misunderstand something, but IMO the first thing that happens is that @Language.factory is run, including the config validation. I don't see how we can precede that or even catch that the resulting error?

[Edit] Well, I guess we can build our own @Language.factory that essentially calls spaCy's @Language.factory, but does those checks before. Not sure we really want that.

Yea, I guess you're right. We could make it a "FAQ" in the discussions forum so at least people will (hopefully) easily find the solution when they get that specific error?

usage_examples/README.md Outdated Show resolved Hide resolved
@svlandeg
Copy link
Member

The readme should somewhere have a list of all models available from llm_models, right? (at least for the non-langchain-ones)

Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

This is looking really good, thanks for pushing this one true Raphael! 🙏

I mainly had some smaller comments, typo's etc. Feel free to merge once you're ready.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
migration_guide.md Outdated Show resolved Hide resolved
migration_guide.md Outdated Show resolved Hide resolved
migration_guide.md Show resolved Hide resolved
spacy_llm/models/rest/cohere/registry.py Show resolved Hide resolved
spacy_llm/models/rest/openai/registry.py Show resolved Hide resolved
spacy_llm/pipeline/llm.py Show resolved Hide resolved
@rmitsch rmitsch merged commit f8244b0 into develop Jun 29, 2023
11 of 12 checks passed
@rmitsch rmitsch deleted the refactor/backend-to-models branch June 29, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring of existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants