Skip to content
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

text-to-image: replace nested dict by height and width properties in the input schema #1158

Merged
merged 5 commits into from
Feb 4, 2025

Conversation

hanouticelina
Copy link
Contributor

Flattening height and width parameters for text-to-image, making the API simpler for users and making provider-specific transformations (dict/enum) easier to handle for us.

yes, It's a breaking change but I expect the usage of target_size to be really minimal so far.

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Just to be sure, it means we need to do the height/width => target_size conversion for "hf-inference" provider, right? Maybe can be done in the same PR for the hf.js client (here).

@hanouticelina
Copy link
Contributor Author

Just to be sure, it means we need to do the height/width => target_size conversion for "hf-inference" provider, right? Maybe can be done in the same PR for the hf.js client (here).

Not sure about that, if the pipeline we're using for hf-inference is this one : https://huggingface.co/docs/diffusers/main/en/api/pipelines/stable_diffusion/text2img, we don't need to do the conversion.

It seems that it's indeed this one that is used for text-to-image : huggingface/api-inference-community/docker_images/diffusers/app/pipelines/text_to_image.py

@Wauplin
Copy link
Contributor

Wauplin commented Jan 31, 2025

Even better! Then, there shouldn't have been a target_size in the first place?

@hanouticelina
Copy link
Contributor Author

Even better! Then, there shouldn't have been a target_size in the first place?

I guess so 😄

@hanouticelina
Copy link
Contributor Author

failing CI is unrelated, it seems like there is a security check failure when trying to install node.js packages

@hanouticelina hanouticelina merged commit 48cd514 into main Feb 4, 2025
5 checks passed
@hanouticelina hanouticelina deleted the update-text-to-image-input-specs branch February 4, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants