openai / openai-python

The official Python library for the OpenAI API
https://pypi.org/project/openai/
Apache License 2.0
21.94k stars 3.02k forks source link

ChatCompletions create() doesn't type-check enums as role #1300

Open npip99 opened 5 months ago

npip99 commented 5 months ago

Confirm this is a feature request for the Python library and not the underlying OpenAI API.

Describe the feature or improvement you're requesting

This was discussed previously @ #911, but just opening a new issue so that it's in the issue tracker.

The Bug

When calling,

role: Literal["system", "user", "assistant"] = ...
completion = await client.chat.completions.create(
    model="gpt-4",
    messages={"role": role, "content": "Hi"}
)

Reason

The main issue is that it's defined as an enum of typed dicts,

ChatCompletionMessageParam = Union[
    ChatCompletionSystemMessageParam,
    ChatCompletionUserMessageParam,
    ChatCompletionAssistantMessageParam,
    ChatCompletionToolMessageParam,
    ChatCompletionFunctionMessageParam,
]

Of course, for tooling & functions, we need to know exactly whether or not it's a user or assistant message. But role: str, content: str is the most common use-case, so it would be nice if system/user/assistant can all use that kind of interface when necessary.

Additional context

Potential Solution

From https://github.com/openai/openai-python/issues/911#issuecomment-2038669860,

I think a very simple solution would be to add another type to the ChatCompletionMessageParam union.

Currently, we have

ChatCompletionMessageParam = Union[
    ChatCompletionSystemMessageParam,
    ChatCompletionUserMessageParam,
    ChatCompletionAssistantMessageParam,
    ChatCompletionToolMessageParam,
    ChatCompletionFunctionMessageParam,
]

If instead we had,

ChatCompletionMessageParam = Union[
    ChatCompletionSystemMessageParam,
    ChatCompletionUserMessageParam,
    ChatCompletionAssistantMessageParam,
    ChatCompletionToolMessageParam,
    ChatCompletionFunctionMessageParam,
    ChatCompletionGenericMessageParam,
]

Where ChatCompletionGenericMessageParam was,

class ChatCompletionGenericMessageParam(TypedDict, total=False):
    content: Required[Optional[str]]
    role: Required[Literal["system", "user", "assistant"]]
npip99 commented 5 months ago

Solution is also provided at https://github.com/openai/openai-python/issues/911#issuecomment-1857142928

It's described as pydantic, but I don't think we can have messages= accept both dictionaries and pydantic models. However, Message can be a function taking 2 kwargs role and user. And, it returns a Union[ChatCompletionSystemMessageParam, ChatCompletionUserMessageParam, ChatCompletionAssistantMessageParam], implemented internally using an if/elif/else.

Maybe name can be CreateChatCompletionMessageParam, not sure.

Just different ideas.


If the reason for making the User have to call a function is because the typing is automatically generated, then maybe there's a way to manually add in a type there that can be brought into the automated system. I feel it will be hard for Users to navigate into this issue and know to use a factory for this type. But, I have no idea, maybe it's not possible to work this into the auto-generated system.

But, having a factory will be nice regardless, because it means you can do my_messages = [CreateMessage(...)], and that my_messages variable will be typed as a list of messages rather than as a list of dicts (Causing mypy errors if you later try to use my_messages).

rattrayalex commented 5 months ago

Thanks for the suggestion. We'll take it into consideration as we think about ways to improve this experience.