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

chore: add missing openai.fm model names to TTSModelSettings.voice type #269

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hironow
Copy link
Contributor

@hironow hironow commented Mar 20, 2025

changes

Include two missing model names ( ballad , verse ) from openai.fm in the voice type definition to ensure coverage of all TTS voices.

Note that a simultaneous update of the dependency module is required for full compatibility—this is the reason why tests are failing.
https://github.com/openai/openai-python/blob/6d0ecdd8ecbface903cf93c7571398b90b803b0b/src/openai/resources/audio/speech.py#L56

@rm-openai
Copy link
Collaborator

looks like tests are failing?

@hironow
Copy link
Contributor Author

hironow commented Mar 21, 2025

Thanks for your review! @rm-openai

I realized that I omitted some details in the description.
It appears that the test failures are due to this process, as explained here (occurred type mismatching):

To clarify, the changes work together with the related PR in openai-python(deps repo) as follows:

  1. The model type is updated in openai-python (deps repo and chore: add missing openai.fm models to voice Literal openai-python#2237 ).
  2. The updated version is then used as a dependency in openai-agents-python (this repo).
  3. Finally, the model type is updated in openai-agents-python (this repo and this PR).

Please note that first, the addition of the model type on the openai-python side must be completed. After that, when openai-agents-python uses the updated version (step 2), there is a concern that the typecheck might still fail.

I appreciate your attention to this matter!

@rm-openai
Copy link
Collaborator

Makes sense. I think we should probably update this param in the agents SDK to be a str | Literal, so that you can pass arbitrary model names

@hironow
Copy link
Contributor Author

hironow commented Mar 21, 2025

That makes sense. However, even with the parameter updated to str | Literal, passing a string now causes an error. Should we wait for an update on the openai-python side, or use a type: ignore workaround for now?

suggestion here:

voice=settings.voice or DEFAULT_VOICE, # type: ignore

@rm-openai
Copy link
Collaborator

rm-openai commented Mar 21, 2025

yes, we'll be updating the openai SDK soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants