Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
else:
return False
try:
model = provider.get_model(model_type, model_name, model_credential)
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.check_auth()
except Exception as e:
if isinstance(e, AppApiException):
Expand All @@ -44,4 +44,4 @@ def encryption_dict(self, model: Dict[str, object]):
return {**model, 'api_key': super().encryption(model.get('api_key', ''))}

def get_model_params_setting_form(self, model_name):
pass
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a few potential issues, optimizations, and improvements to consider:

  1. Optimize get_model call: The current implementation of the get_model call passes three parameters (model_type, model_name, and model_credential). If additional keyword arguments may be needed in the future (such as **kwargs), it might make sense to update this method to handle them properly.

  2. Clarify pass statement: In the get_model_params_setting_form function, the pass statement could indicate some functionality that should be implemented later on. It would be better to remove return: or replace it with an actual implementation when necessary.

  3. Consider moving validation logic elsewhere: Currently, all the validation logic is mixed into the main body of the class's methods. This can lead to duplication and reduce readability. Consider extracting these validations to separate functions or classes where they belong more logically.

Here's the updated code snippet incorporating suggested changes:

def __init__(self, *args, **kwargs):
    # Initialize instance variables here

def is_valid(self, model_type: str, model_name, model_credential: Dict[str, object]):
    if not self._provider.get_provider_status():
        return False
    params = {'model_type': model_type, "key": model_credential['api_key'], 'name': model_name}
    try:
        model_data = self._provider.get_model(params)  # Use named arguments instead
        model.check_auth()  # Ensure authentication before further processing
    except Exception as e:
        if isinstance(e, AppApiException):
            raise e
        return False

def encryption_dict(self, model: Dict[str, object]):
    encrypted_dict = {**model, 'api_key': self._encryption.encrypt(model.get('api_key', ''))}
    return encrypted_dict

def get_model_params_setting_form(self, model_name):
    # Implement form creation or settings configuration for specific models if required
    pass

These changes aim to improve clarity, maintainability, and flexibility in the code while addressing potential runtime errors related to missing keywords and redundant checks.

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class XInferenceSpeechToText(MaxKBBaseModel, BaseSpeechToText):
api_base: str
api_key: str
model: str
params: dict

def __init__(self, **kwargs):
super().__init__(**kwargs)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet for XInferenceSpeechToText is missing some important attributes. The params attribute is defined within the class definition but is not initialized anywhere during object creation or usage.

Here's an updated version of the class with corrections and additional comments:

from abc import ABC, abstractmethod
import max_kb_base_model   # Assuming this module exists

class MaxKBBaseModel(ABC):    
    @abstractmethod
    def process(self):
        pass    

class BaseSpeechToText(ABC):   
    @abstractmethod
    def transcribe(self, audio_file):
        pass    

class XInferenceSpeechToText(MaxKBBaseModel, BaseSpeechToText):
    api_base: str
    api_key: str
    model: str
    params: dict  # Initialize params in __init__ method
    
    def __init__(self, **kwargs):
        super().__init__()
        
        self.api_base = kwargs.get('api_base', 'default_url')
        self.api_key = kwargs.get('api_key', '')
        self.model = kwargs.get('model', 'default_model')
        self.params = kwargs.get('params', {})  # Default to an empty dictionary if none provided
        
        print(f"Initialized {type(self).__name__} instance with API base: '{self.api_base}', "
              f"API key: '{self.api_key}', model: '{self.model}'")

# Example usage:
# inference_stt = XInferenceSpeechToText(api_base='https://example.com/api', 
                                       # api_key='your_api_key',
                                       # model='inception_v3')

Key Improvements:

  1. Initialization of params: Added an initialization line in the __init__ method that assigns a default value to self.params.
  2. Default Values: Set defaults for each parameter (api_base, api_key, model) using kwargs, which allows flexibility when creating instances (e.g., via function arguments).
  3. Error Handling: Used kwargs.get() to safely access parameters, setting sane default values where needed.
  4. Print Statement: Included a message in the constructor to confirm initialization details.

This should address any irregularities present in the original code. Let me know if you have any further questions!

Expand Down
Loading