-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for multi-modal moderation inputs and category applied input types #1
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
Conversation
tests/Resources/Moderations.php
Outdated
test('create omni', closure: function () { | ||
$client = mockClient('POST', 'moderations', [ | ||
'model' => 'omni-moderation-latest', | ||
'input' => 'I want to kill them.', | ||
'input' => [ | ||
['type' => 'text', 'text' => '.. I want to kill...'], | ||
], | ||
], Response::from(moderationOmniResource(), metaHeaders())); | ||
|
||
$result = $client->moderations()->create([ | ||
'model' => 'omni-moderation-latest', | ||
'input' => 'I want to kill them.', | ||
'input' => [ | ||
['type' => 'text', 'text' => '.. I want to kill...'], | ||
], | ||
]); |
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 should probably support both right? We need to support the string
legacy option as well as array of multi-modal
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.
@iBotPeaches Yes, It support both. Should I revert this changes and create new unit test ?
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.
I just want to make sure both work. You changed both of them to the multi-modal approach. We should probably have:
- A basic text only test
- A basic text & image test
- A basic text in array test
Looks good. Please open this PR upstream on the main repo for the other maintainers. So repeat yourself a bit in the description of the PR to remind why we are adding this. Don't leak any clients though. |
Merged upstream - openai-php@62e9822 |
What:
Description:
Related: