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.

Sign up for GitHub

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

Jump to bottom

Standardize on using image argument in all pipelines #1361

Merged
merged 6 commits into from Dec 1, 2022
Merged

Standardize on using image argument in all pipelines #1361

merged 6 commits into from Dec 1, 2022

Conversation

fboulnois
Copy link
Contributor

This standardizes the use of the argument image in all pipelines instead of a mix of init_image and image.

Resolves #1257

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Nov 21, 2022 •
edited

The documentation is not available anymore as the PR was closed or merged.

@fboulnois fboulnois changed the title Standardize on using image in all pipelines Standardize on using image argument in all pipelines Nov 21, 2022
@fboulnois fboulnois marked this pull request as ready for review November 21, 2022 19:13
@pcuenca pcuenca mentioned this pull request Nov 23, 2022
Merged
@averad averad mentioned this pull request Nov 24, 2022
Merged
patrickvonplaten
patrickvonplaten reviewed Nov 28, 2022
View reviewed changes
src/diffusers/pipelines/latent_diffusion/pipeline_latent_diffusion_superresolution.py
@@ -66,7 +66,7 @@ def __init__(
@torch.no_grad()
def __call__(
self,
init_image: Union[torch.Tensor, PIL.Image.Image],
image: Union[torch.Tensor, PIL.Image.Image],
Copy link
Contributor

Choose a reason for hiding this comment

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

we will have to deprecate init_image here to not surprise the user here too much. Could you implement the same logic we're using for schedulers for all pipelines:

diffusers/src/diffusers/pipelines/ddpm/pipeline_ddpm.py

Line 76 in 5755d16

predict_epsilon = deprecate("predict_epsilon", "0.10.0", message, take_from=kwargs)

Here would should do the following in addition to renaming init_image to image:

  1. Add a **kwargs argument to the function call
 message = (
    "Please make sure to use `image` instead of `init_image`."
)
init_image = deprecate("init_image", "0.12.0", message, take_from=kwargs)
image = init_image or image

Copy link
Contributor

Choose a reason for hiding this comment

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

We should do this for all pipelines :-)

patrickvonplaten
patrickvonplaten reviewed Nov 28, 2022
View reviewed changes
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Super nice PR @fboulnois !

Everything looks very nice to me except for deprecating init_image correctly.
Could you try to implement https://github.com/huggingface/diffusers/pull/1361/files#r1033477098 for all pipelines (src/pipelines) + community?

Let me know if you need any help :-)

@fboulnois
Copy link
Contributor Author

@patrickvonplaten Done! Rebased on main, added deprecation warnings to community and core pipelines, and all tests pass.

patrickvonplaten
patrickvonplaten reviewed Dec 1, 2022
View reviewed changes
src/diffusers/pipelines/alt_diffusion/pipeline_alt_diffusion_img2img.py
@@ -540,6 +541,10 @@ def __call__(
list of `bool`s denoting whether the corresponding generated image likely represents "not-safe-for-work"
(nsfw) content, according to the `safety_checker`.
"""
message = "Please use `image` instead of `init_image`."
init_image = deprecate("init_image", "0.12.0", message, take_from=kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

great!

patrickvonplaten
patrickvonplaten approved these changes Dec 1, 2022
View reviewed changes
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

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

Super clean! Great job @fboulnois 🥳

Merging :-)

@patrickvonplaten patrickvonplaten merged commit 52eb034 into huggingface:main Dec 1, 2022
@fboulnois fboulnois deleted the switch-init-image-arg branch December 1, 2022 16:04
@pcuenca pcuenca mentioned this pull request Dec 3, 2022
Merged
2 tasks
tcapelle pushed a commit to tcapelle/diffusers that referenced this pull request Dec 12, 2022
@fboulnois
Standardize on using image argument in all pipelines (huggingface#1361
210bab3 
)

* feat: switch core pipelines to use image arg

* test: update tests for core pipelines

* feat: switch examples to use image arg

* docs: update docs to use image arg

* style: format code using black and doc-builder

* fix: deprecate use of init_image in all pipelines
sliard pushed a commit to sliard/diffusers that referenced this pull request Dec 21, 2022
@fboulnois @sliard
Standardize on using image argument in all pipelines (huggingface#1361
d8f5d43 
)

* feat: switch core pipelines to use image arg

* test: update tests for core pipelines

* feat: switch examples to use image arg

* docs: update docs to use image arg

* style: format code using black and doc-builder

* fix: deprecate use of init_image in all pipelines
@fboulnois fboulnois mentioned this pull request Dec 27, 2022
Closed
@pcuenca pcuenca mentioned this pull request Feb 1, 2023
Closed
yoonseokjin pushed a commit to yoonseokjin/diffusers that referenced this pull request Dec 25, 2023
@fboulnois
Standardize on using image argument in all pipelines (huggingface#1361
d4f0742 
)

* feat: switch core pipelines to use image arg

* test: update tests for core pipelines

* feat: switch examples to use image arg

* docs: update docs to use image arg

* style: format code using black and doc-builder

* fix: deprecate use of init_image in all pipelines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrickvonplaten patrickvonplaten patrickvonplaten approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Standardize on init_image for img2img, inpaint, and others
3 participants
@fboulnois @HuggingFaceDocBuilderDev @patrickvonplaten