Skip to content

Remove conversion to RGB - #6479

Merged
sayakpaul merged 11 commits into
huggingface:mainfrom
yelboudouri:main
Jan 12, 2024
Merged

Remove conversion to RGB#6479
sayakpaul merged 11 commits into
huggingface:mainfrom
yelboudouri:main

Conversation

@yelboudouri

Copy link
Copy Markdown
Contributor

What does this PR do?

I removed the conversion to RGB in src/diffusers/utils/loading_utils.py. I spent hours trying to figure out why my RGBA images were getting a black background upon loading. I then realized that this function internally converts images to RGB, which should not be the case because this function is only responsible for loading the image. It's up to the user to decide which mode they want to convert their images to after loading them.

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@patrickvonplaten

Copy link
Copy Markdown
Contributor

Hey @yelboudouri,

This change would be a breaking change - could you add a parameter: "return_type='....'" to the function that allows to load RGBA images?

@yelboudouri

yelboudouri commented Jan 9, 2024

Copy link
Copy Markdown
ContributorAuthor

I'm thinking of a way to solve this problem without breaking the code and also solving it once and for all because adding a return_type parameter is not sufficient. Image.convert() has more parameters than just the mode to convert to. So, I'm thinking about adding a parameter convert_method defaulting to None, and then if the convert method is provided, apply it to the image.

The updated code will be something like:

fromtypingimportUnionimportosimportrequestsimportPIL.ImageimportPIL.ImageOpsdefload_image(image: Union[str, PIL.Image.Image], convert_method=None) ->PIL.Image.Image:
""" Loads `image` to a PIL Image. Args: image (`str` or `PIL.Image.Image`): The image to convert to the PIL Image format. convert_method (`function`, optional): The conversion method for converting the image to RGB. Default is None. Returns: `PIL.Image.Image`: A PIL Image. """ifisinstance(image, str):
ifimage.startswith("http://") orimage.startswith("https://"):
image=PIL.Image.open(requests.get(image, stream=True).raw)
elifos.path.isfile(image):
image=PIL.Image.open(image)
else:
raiseValueError(
f"Incorrect path or URL. URLs must start with `http://` or `https://`, and {image} is not a valid path."
)
elifisinstance(image, PIL.Image.Image):
passelse:
raiseValueError(
"Incorrect format used for the image. Should be a URL linking to an image, a local path, or a PIL image."
)
image=PIL.ImageOps.exif_transpose(image)
# Check if a custom convert method is providedifconvert_methodisnotNone:
image=convert_method(image)
else:
image=image.convert('RGB')
returnimage

And when using it:

defcustom_convert_method(img):
returnimg.convert('L', colors=8)
loaded_image=load_image("image.jpg", convert_method=custom_convert_method)

What do you think @patrickvonplaten ?

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Comment threadsrc/diffusers/utils/loading_utils.py Outdated
yelboudouriand others added 2 commits January 11, 2024 15:08
Update docstring
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

@sayakpaulsayakpaul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Super! Thanks for this!

@sayakpaul

Copy link
Copy Markdown
Member

Let's get the code quality issues fixed.

DN6
DN6 approved these changes Jan 11, 2024

@DN6DN6 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 👍🏽 Let's just fix code QC issues.

@yelboudouri

Copy link
Copy Markdown
ContributorAuthor

Can't get that QC to pass, it says 'Import block is unsorted or unformatted,' but the imports are already sorted.

@yiyixuxu

Copy link
Copy Markdown
Collaborator

@yelboudouri did you run make style

@yelboudouri

Copy link
Copy Markdown
ContributorAuthor

@yiyixuxu forgot that 🤦‍♂️ Now it should be all good.

@sayakpaul

Copy link
Copy Markdown
Member

Thank you for your contributions!

@sayakpaul
sayakpaul merged commit c6b0458 into huggingface:mainJan 12, 2024
@Laisky

Laisky commented Feb 8, 2024

Copy link
Copy Markdown
Contributor

This PR has caused a breaking change. Does this mean that load_image will no longer support PIL.Image? This clearly contradicts the function's comments and errors.

@yelboudouri@patrickvonplaten

fix: #6904

@LaiskyLaisky mentioned this pull request Feb 8, 2024
6 tasks
AmericanPresidentJimmyCarter pushed a commit to AmericanPresidentJimmyCarter/diffusers that referenced this pull request Apr 26, 2024
* Remove conversion to RGB
* Add a Conversion Function
* Add type hint for convert_method
* Update src/diffusers/utils/loading_utils.py
Update docstring
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
* Update docstring
* Optimize imports
* Optimize imports (2)
* Reformat code
---------
Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>
Sign up for freeto 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.

7 participants

@yelboudouri@patrickvonplaten@HuggingFaceDocBuilderDev@sayakpaul@yiyixuxu@Laisky@DN6