Skip to content

Conversation

@gzileni
Copy link

@gzileni gzileni commented Apr 21, 2025

This pull request introduces a significant refactor across multiple files to make various classes in the vanna package abstract by inheriting from Python’s ABC (Abstract Base Class). Additionally, it includes enhancements to the OpenAI_Chat class to improve configurability and default behavior. Below are the key changes grouped by theme:

Refactoring to Abstract Base Classes:

  • Updated numerous classes to inherit from ABC to indicate that they are abstract and may include abstract methods. This change was applied to classes such as ZhipuAI_Chat, ZhipuAI_Embeddings, Cohere_Chat, Cohere_Embeddings, DeepSeekChat, FAISS, BigQuery_VectorStore, GoogleGeminiChat, Hf, Marqo_VectorStore, Mistral, MockEmbedding, MockLLM, OpenAI_Embeddings, OpenSearch_VectorStore, and OpenSearch_Semantic_VectorStore. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]

Enhancements to OpenAI_Chat:

  • OpenAI_Chat now inherits from ABC and includes new attributes (temperature, client, model, max_retries) with default values.
  • Improved configurability by allowing max_retries and model to be set via the configuration. Defaults to gpt-3.5-turbo if no model is specified. [1] [2]
  • Adjusted the __init__ method to include max_retries when initializing the OpenAI client.
  • Enhanced the submit_prompt method to handle default model selection more robustly and clarified comments.

Cleanup and Minor Adjustments:

This refactor improves the codebase by enforcing abstraction where appropriate, enhancing clarity, and making the OpenAI_Chat class more flexible and robust.

Copy link

@llamapreview llamapreview bot left a comment

Choose a reason for hiding this comment

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

Auto Pull Request Review from LlamaPReview

Large PR Notification

Dear contributor,

Thank you for your substantial contribution to this project. LlamaPReview has detected that this Pull Request contains a large volume of changes, which exceeds our current processing capacity.

Details:

  • PR and related contents total size: Approximately 67,251 characters
  • Current limit: 50,000 characters

Next steps:

  1. Consider breaking this PR into smaller, more focused changes if possible.
  2. For manual review, please reach out to your team members or maintainers.

We appreciate your understanding and commitment to improving this project. Your contributions are valuable, and we want to ensure they receive the attention they deserve.

LlamaPReview is continuously evolving to better serve the community. Share your thoughts on handling large PRs in our GitHub Discussions - your feedback helps us improve and expand our capabilities.

If you have any questions or need assistance, our community and support team are here to help.

Best regards,
LlamaPReview Team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant