-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Fix] Add chat completion Example and simplify dependencies #576
Conversation
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.
Thanks for the PR! Left minor comments.
examples/openai_client.py
Outdated
# Chat completion API | ||
chat_completion = openai.ChatCompletion.create( | ||
model=model, | ||
messages=[{ | ||
"role": "system", | ||
"content": "You are a helpful assistant." | ||
}, { | ||
"role": "user", | ||
"content": "Who won the world series in 2020?" | ||
}, { | ||
"role": | ||
"assistant", | ||
"content": | ||
"The Los Angeles Dodgers won the World Series in 2020." | ||
}, { | ||
"role": "user", | ||
"content": "Where was it played?" | ||
}]) | ||
|
||
print("Chat completion results:") | ||
print(chat_completion) |
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.
A quick question: why don't we have this in a separate file (say openai_chat_client.py
)?
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.
No special reason at the beginning. Let me move it to another file then.
@@ -63,6 +67,9 @@ async def check_model(request) -> Optional[JSONResponse]: | |||
|
|||
|
|||
async def get_gen_prompt(request) -> str: | |||
assert _fastchat_available, ( |
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.
nit: As this is user-facing error, I feel raising error is a bit more appropriate.
assert is used to ensure internal correctness, not to enforce correct usage nor to indicate that some unexpected event occurred. If an exception is desired in the latter cases, use a raise statement.
Fix #545 #537
Make fastchat an optional dependency giving the huge dependency list of fastchat and many complaints on installation.