openai-python icon indicating copy to clipboard operation
openai-python copied to clipboard

Mypy typing error when treating a ChatCompletionSystemMessageParam as dict

Open pamelafox opened this issue 1 year ago • 6 comments

Confirm this is an issue with the Python library and not an underlying OpenAI API

  • [X] This is an issue with the Python library

Describe the bug

We have a list like:

messages: list[ChatCompletionMessageParam]

We then convert a message to a dict:

dict(messages[0])

Mypy does not like that:

Argument 1 to "dict" has incompatible type "ChatCompletionSystemMessageParam | ChatCompletionUserMessageParam | ChatCompletionAssistantMessageParam | ChatCompletionToolMessageParam | ChatCompletionFunctionMessageParam"; expected "SupportsKeysAndGetItem[str, str]" [arg-type]

I'm not sure if this is a bug with mypy or with the SDK, to be honest. Or maybe there's even a better approach in our code. I see that your classes extend TypedDict, so I'd think that should be considered as SupportsKeysAndGetItem, but I'm probably missing something.

To Reproduce

See code above. You can also see our CI failing here: https://github.com/Azure-Samples/azure-search-openai-demo/actions/runs/7786990746/job/21233073297?pr=1233

Code snippets

No response

OS

MacOS

Python version

3.11

Library version

1.10.0

pamelafox avatar Feb 05 '24 17:02 pamelafox

Hey thanks for the report @pamelafox ! I think this is sort of "just how mypy works" but we are exploring ways to make this sort of situation better in the near term (cc @RobertCraigie) so hopefully we'll have an update / follow-up questions or proposals to share within a week or two.

rattrayalex avatar Feb 05 '24 18:02 rattrayalex

Unfortunately this looks to just be a mypy bug. Pyright doesn't report any errors in this case.

Note that in general I would strongly recommend just relying on Pyright as bugs and features are implemented at a much faster rate compared to mypy.

I am curious why you're converting a message param into a dictionary, isn't it already a dictionary?

RobertCraigie avatar Feb 05 '24 18:02 RobertCraigie

Is there a mypy bug pertaining to this particular situation? I found related issues but didn't see an obvious one. Let me know if I should file one.

I agree that we shouldn't have to wrap in dict, I think that was just our attempt to make mypy happy (which didn't work). We are passing it into a function that accepts a dict, and it has the same error if we remove the dict() as well. The error will either be on dict() if we wrap it, or our internal count_tokens(message:dict) function if not.

pamelafox avatar Feb 05 '24 22:02 pamelafox

We can look into pyright, we've been using mypy for more of our projects. Pyright makes sense given we're from Microsoft but mypy seems to be more popular with Python community.

pamelafox avatar Feb 05 '24 22:02 pamelafox

Ah @pamelafox, you can't pass TypedDicts to functions that expect a dict because that dictionary could be mutated to include keys that aren't present on the TypedDict or other similar issues.

You have two options that I can see:

  • Use Mapping[str, object] if you dont do any mutation
  • Reference our types directly

I'd personally recommend referencing our types directly if possible, it'll be much more maintainable long term.

RobertCraigie avatar Feb 05 '24 23:02 RobertCraigie

Ah, great point! Mapping[str, object] works for those functions (they do not mutate) and makes mypy happy. Feel free to close this, unless there are additional improvements planned.

By the way, I just looked up DL numbers for mypy vs pyright, and its 5 million weekly vs 400K weekly, so we might move to pyright ourselves but I expect much of the community will still use mypy.

pamelafox avatar Feb 06 '24 15:02 pamelafox