-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: STT model params #4047
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
feat: STT model params #4047
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
||
| def get_model_params_setting_form(self, model_name): | ||
| pass | ||
| return TencentSSTModelParams() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided code snippet looks largely intact and contains no immediate syntax errors or logical issues, but there are some improvements that can be made:
-
Variable Names: Ensure consistency in variable names throughout the file to improve readability.
-
Comments Formatting: The comments follow snake_case format which is more common in Python than underscore_separated_names like
EngSerViceType. Convert them accordingly if desired. -
Docstring Consistency: It's beneficial to maintain consistent docstrings across functions and classes. Add appropriate comments explaining what each method does.
Here’s a revised version of the code with improved naming conventions and additional comments:
from models_provider.base_model_provider import BaseModelCredential, ValidCode
class TencentSSTModelParams(BaseForm):
eng_service_type = forms.SingleSelect(
TooltipLabel(_('Engine model type'), _('If not passed, the default value is 16k_zh (Chinese universal)')),
required=True,
default_value='16k_zh',
option_list=[
{"value": "8k_zh", "label": _("Chinese telephone universal")},
{"value": "8k_en", "label": _("English telephone universal")},
{"value": "16k_zh", "label": _("Commonly used in Chinese")},
{"value": "16k_zh-py", "label": _("Chinese, English, and Guangdong")},
{"value": "16k_zh_medical", "label": _("Chinese medical")},
{"value": "16k_en", "label": _("English")},
{"value": "16k_yue", "label": _("Cantonese")},
{"value": "16k_ja", "label": _("Japanese")},
{"value": "16k_ko", "label": _("Korean")},
{"value": "16k_vi", "label": _("Vietnamese")},
{"value": "16k_ms", "label": _("Malay language")},
{"value": "16k_id", "label": _("Indonesian language")},
{"value": "16k_fil", "label": _("Filipino language")},
{"value": "16k_th", "label": _(" Thai")},
{"value": "16k_pt", "label": _("Portuguese")},
{"value": "16k_tr", "label": _("Turkish")},
{"value": "16k_ar", "label": _("Arabic")},
{"value": "16k_es", "label": _("Spanish")},
{"value": "16k_hi", "label": _("Hindi")},
{"value": "16k_fr", "label": _("French")},
{"value": "16k_de", "label": _("German")},
{"value": "16k_zh_dialect", "label": _("Multiple dialects, supporting 23 dialects")}
],
value_field="value",
text_field="label"
)
class TencentSTTModelCredential(BaseForm, BaseModelCredential):
REQUIRED_FIELDS = ["secret_id", "secret_key"]
secret_id = forms.CharField('secret_id', required=True)
secret_key = forms.PasswordInputField('secret_key', required=True)
def get_model_params_setting_form(self, model_name):
# Retrieve and return specific configuration form for the given model
return TencentSSTModelParams()Additional Remarks:
- Use lowercase
eng_service_typeinstead ofEngSerViceType. - Add an empty line after method definitions and before function calls for better formatting.
- Consider adding a comment at the top of the file to provide context about what the module or class does.
These changes make the code cleaner and easier to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no immediate irregularities or major issues with the provided code snippet. However, there's a small improvement you can make to enhance readability:
from models_provider.base_model_provider import BaseModelCredential, ValidCode
from django.utils.translation import gettext_lazy as _
class AliyunBaiLianOmiSTTModelParams(forms.Form): # Change base class from BaseForm to forms.Form
CueWord = forms.TextInputField(
tooltip_label=_('CueWord'),
help_text=_("If not passed, the default value is '这段音频在说什么,只回答音频的内容'"),
required=True,
)
class AliyunBaiLianOmiSTTModelCredential(forms.ModelForm, BaseModelCredential): # Add ModelForm mixin
credential_type_code_choices = (
('ali_bailian_omi_stt', "Aliyun BaiLian OMI STT Credential"),
) # Ensure this part isn't missing
def encryption_dict(self, model: Dict[str, object]) -> Dict[str, object]:
pass
def get_model_params_setting_form(self, model_name):
# If you need additional customization for form fields, add them here.
return AliyunBaiLianOmiSTTModelParams()Changes Made:
-
Base Class Update: Changed
BaseForminAliyunBaiLianOmiSTTModelParamstoforms.Form. This change is standard practice when using Django's built-in form library. -
Attributes Formatting: Updated
TooltipLabelandrequired=Trueto use Django's preferred attribute names (tooltip_label,help_text) and improved their formatting slightly for better clarity and consistency. -
ModelForm Mixin: Added the
forms.ModelFormmix intoAliyunBaiLianOmiSTTModelCredential, which provides some convenience methods (likecleaned_data) and allows handling of related models more directly by automatically creating a form tied to the model instance. -
Additional Documentation: Provided an example on how to customize the form further if needed.
These changes should make the code more readable and align closely with best practices used in Django development.
| {"type": "text", "text": self.params.get('CueWord')}, | ||
| ], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided code snippet seems to be part of an API request that sends a transcribed text message along with an audio file. Some potential issues and optimizations:
-
Parameter Usage: The
self.paramsdictionary is used without checking if it contains the key'CueWord'. This could lead to KeyError if the parameter is not present. -
Text Content: The text "这段音频在说什么,只回答音频的内容" is hardcoded within the response. If you want to dynamically inject this text based on the value from the parameters, adjust accordingly.
-
Optimization: Ensure that all necessary environment variables or dependencies (like Google Cloud Speech-to-Text API client) are properly set up and initialized before making requests. Also, consider using asynchronous programming if dealing with multiple requests at once.
Here's an example of how you might modify the function to handle these considerations:
def speech_to_text(self, audio_file):
# Assuming params is already defined and contains necessary configurations
prompt = {
"language_code": "zh-CN",
"media_content_type": "audio/mp3",
"input": {
"content": open(audio_file, 'rb').read(),
},
"output_config": {
"audio_encoding": "LINEAR16", # Adjust encoding according to your needs
},
}
return prompt
# Example usage:
# prompt_dict = my_instance.speech_to_text("path/to/audio.mp3")Key Points:
- Check for Parameter Existence: Before accessing
params.get('CueWord'), check its presence. - Dynamic Content: Replace static content with dynamic references to parameters where appropriate.
- Environment Setup: Verify and initialize dependencies needed for API calls.
- Asynchronous Processing: Consider asynchronous processing to improve performance when handling multiple requests concurrently.
What this PR does / why we need it?
Summary of your change
Please indicate you've done the following: