Skip to content

Use child model classes as factories instead #301

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JoshuaBehrens
Copy link
Contributor

This commit bricks several instanceof checks. Other changes needs to be done as well. See the other pull requests:

Q A
Bug fix? "yes"
New feature? no
Docs? no
Issues Fix #114
License MIT

In other issues and pull requests the model class should be used as a simple struct, only holding scalar values and should not have any business logic effect by inheritance. This has been the case for embeddings, completions and other API calls like Whisper audio, and Dall-e images. Right now this makes it difficult to share different instances of the model class with various providers, although a lot of providers share the same models, and therefore likely the same capabilities. This eventually will reduce the amount of code you have to write to establish new providers that replicate existing API's with a well-known subset of models or add better support for custom models like OpenAI fine-tuned models or ollamas self-built models.

This is pull request is about to suggest a way to reflector the usage of the model classes to only use the model class as a struct without using inheritance. I stick to the previous classes to simplify code changes and the current effects of different types of models being grouped in a class. The class hierarchy has been broken up, so the previously sub classes of the model class are no longer extending the model class, and the model of the class has been made final to prevent changes like this in the future. Right now, it will break instanceof checks in several classes and can only be merged if the linked requests merged. It is not part of the others, as the approach how to solve this particular issue can be discussed separately.

This commit bricks several instanceof checks. Other changes needs to be done as well
@carsonbot carsonbot changed the title Use child model classes as factories instead Use child model classes as factories instead Aug 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom URL ability or generic bridges?
2 participants