-
Notifications
You must be signed in to change notification settings - Fork 79
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
[extensions] 3\n Image 2 text #940
Conversation
fa9f45e
to
cdce717
Compare
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the ASR Model parser to validate input is in the expected form/type. Also made some updates to to make input attachment validation clearer. ## Testplan <img width="1312" alt="Screenshot 2024-01-16 at 3 29 02 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/f3574860-7d1b-4768-a536-7c6cd875af3c"> ### Dependencies Sapling removed the dependency pr. Depends on #929
955d4a3
to
3ff8b60
Compare
With a new input attachment type `AttachmentDataWithStringValue` defined, This diff updates the HF image to text Model parser to validate input is in the expected form/type. Also made some updates to to make input attachment validation clearer. ## Testplan <img width="1167" alt="Screenshot 2024-01-16 at 3 32 52 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/bfb6af75-a924-44d4-9a66-25374814d569"> ### Dependencies Sapling removed the dependency pr. Depends on #929
3ff8b60
to
e51ce3e
Compare
@@ -248,18 +249,16 @@ def validate_and_retrieve_images_from_attachments(prompt: Prompt) -> list[Union[ | |||
for i, attachment in enumerate(prompt.input.attachments): | |||
validate_attachment_type_is_image(prompt.name, attachment) | |||
|
|||
input_data = attachment.data |
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: We're calling attachment.data
multiple times, so why not keep this? In L257 we can call it image_source
or something
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.
shipping this as is; will look into this next time I touch this as that may be soon
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.
minor nit but otherwise lgtm
) [editor] 4/n refactor input attachments to use new input data type - update renderer to acknowledge new type - On New attachment, update aiconfig to use new values to input data when new file is updated - updated Gradio AIConfig by removing and adding files. Note: This diff doesn't test or change base64 functionality but should techncially be possible. <audio> HTML taglets you pass base64 source. Unsure about image ## Testplan: 1. edit config 2. reupload files & run. note: the incorrect rendering is due to the old aiconfig with the now incorrect input attachment data type https://github.com/lastmile-ai/aiconfig/assets/141073967/eff42748-e3e6-48df-b90b-34bac4e997c4 --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/944). * __->__ #944 * #940 * #932
[extensions] 3\n Image 2 text
With a new input attachment type
AttachmentDataWithStringValue
defined, This diff updates the HF image to text Model parser to validate input is in the expected form/type.Also made some updates to to make input attachment validation clearer.
Testplan
Dependencies
Sapling removed the dependency pr. Depends on
#929
Stack created with Sapling. Best reviewed with ReviewStack.