-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Add default value to the logprobs parameter #1044
Add default value to the logprobs parameter #1044
Conversation
I encourage merging this; we don't anticipate adding (I'm a maintainer of the openai-python library). |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1044 +/- ##
===========================================
+ Coverage 30.25% 40.65% +10.39%
===========================================
Files 30 30
Lines 3983 3980 -3
Branches 898 946 +48
===========================================
+ Hits 1205 1618 +413
+ Misses 2699 2233 -466
- Partials 79 129 +50
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Looks good to me
@@ -14,7 +14,7 @@ | |||
__version__ = version["__version__"] | |||
|
|||
install_requires = [ | |||
"openai>=1,<1.5", # a temporary fix for breaking changes in 1.5 | |||
"openai>=1.3,<1.5", |
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.
shouldn't we remove "<1.5" if the PR is supposed to support openai>=1.5?
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.
Correct. Somehow this might have been a merge error
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.
Yes, that's probably due to merge. I'll run tests again against the latest 1.6.1 and create a new PR.
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.
Fix dependency error introduced when merging #1044
Fix dependency error introduced when merging #1044
Co-authored-by: Pratyay Roy <[email protected]>
* added default value to logprobs param * fixed to work withversions of openai below 1.5.0 * rebase * fixed openai version in comments * polishing * limit openai version to below 1.5.0 --------- Co-authored-by: Eric Zhu <[email protected]>
Fix dependency error introduced when merging microsoft#1044
Why are these changes needed?
Describe the bug
openai/openai-python#980 added token
logprobs
to chat completions of typeOptional[ChoiceLogprobs]
in openai.types.chat.chat_completion.Choice and openai.types.chat.chat_completion_chunk.Choice. In the latter, the default value is set toNone
, while in the former it is not set. This causes backward compatibility problems with code written for versions prior to 1.5.0.Steps to reproduce
Tests are failing locally and in CI (e.g. https://github.com/microsoft/autogen/actions/runs/7293999612/job/19878098426)
Additional Information
autogen version: 0.2.2
openai version: 1.5.0 and 1.6.0
Related issue number
Closes #1043
Checks