-
Notifications
You must be signed in to change notification settings - Fork 833
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: OpenAIChatCompletion class method shouldSkip method to skip row if no user message #2320
base: master
Are you sure you want to change the base?
Conversation
…ned as there might OpenAIMessages with empty contents. These rows can be identified before sending requests to OpenAI service and getting errors. For now, we are skipping any rows that do not have any user message
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2320 +/- ##
==========================================
+ Coverage 84.44% 84.47% +0.02%
==========================================
Files 328 328
Lines 16771 16776 +5
Branches 1506 1520 +14
==========================================
+ Hits 14163 14171 +8
+ Misses 2608 2605 -3 ☔ View full report in Codecov by Sentry. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
@@ -46,8 +46,22 @@ class OpenAIChatCompletion(override val uid: String) extends OpenAIServicesBase( | |||
|
|||
override val subscriptionKeyHeaderName: String = "api-key" | |||
|
|||
override def shouldSkip(row: Row): Boolean = | |||
super.shouldSkip(row) || Option(row.getAs[Row](getMessagesCol)).isEmpty | |||
/** |
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.
This seems like something we should likely have in the prompt API as opposed to the core API just because im not sure if its possible to send just system messages and have it reply
OpenAIChatCompletion
class methodshouldSkip
was not working as designed as there might OpenAIMessages with empty contents. These rows can be identified before sending requests to OpenAI service and getting errors.What changes are proposed in this pull request?
In this PR, I am making changes to skip rows if there are no user messages in the message column.
How is this patch tested?
Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?