Add improved handling of pil #1309
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
pcuenca
left a comment
pcuenca
left a comment
There was a problem hiding this comment.
There are a few changes about latent diffusion that are not related to PIL as far as I can tell, but I'm ok with that :)
| # 2. once modified, run: `make deps_table_update` to update src/diffusers/dependency_versions_table.py | ||
| _deps = [ | ||
| "Pillow<10.0", # keep the PIL.Image.Resampling deprecation away | ||
| "Pillow", # keep the PIL.Image.Resampling deprecation away |
There was a problem hiding this comment.
| "Pillow", # keep the PIL.Image.Resampling deprecation away | |
| "Pillow", |
| if version.parse(version.parse(PIL.__version__).base_version) >= version.parse("9.1.0"): | ||
| PIL_INTERPOLATION = { | ||
| "linear": PIL.Image.Resampling.BILINEAR, | ||
| "bilinear": PIL.Image.Resampling.BILINEAR, | ||
| "bicubic": PIL.Image.Resampling.BICUBIC, | ||
| "lanczos": PIL.Image.Resampling.LANCZOS, | ||
| "nearest": PIL.Image.Resampling.NEAREST, | ||
| } | ||
| else: | ||
| PIL_INTERPOLATION = { | ||
| "linear": PIL.Image.LINEAR, | ||
| "bilinear": PIL.Image.BILINEAR, | ||
| "bicubic": PIL.Image.BICUBIC, | ||
| "lanczos": PIL.Image.LANCZOS, | ||
| "nearest": PIL.Image.NEAREST, | ||
| } |
There was a problem hiding this comment.
I'd prefer these to be symbols to prevent mistakes and allow autocompletion. Can we use Enums maybe?
There was a problem hiding this comment.
Makes sense!
There was a problem hiding this comment.
Actually, no this will lead to a weird API then because they are functions that are not used to compare anything. I don't want to add if ... comparisons everywhere in the code. Think it's actually fine as is.
E.g. If I do:
class PIL_INTERPOLATION(Enum):
LINEAR = PIL.Image.Resampling.BILINEARThen one cannot call the function with PIL_INTERPOLATION.LINEAR only with PIL_INTERPOLATION.LINEAR.value which is weird. Actually keen to leave as is here -> we have the same logic in transformers for the activation functions: https://github.com/huggingface/transformers/blob/4fb34de99e800689862f80cd618e6468fcb7b28b/src/transformers/activations.py#L152
There was a problem hiding this comment.
Oh, that's weird, I thought it would work without .value. That's too bad, let's keep them as strings then.
|
|
||
| ## LDMTextToImagePipeline | ||
| [[autodoc]] pipelines.latent_diffusion.pipeline_latent_diffusion.LDMTextToImagePipeline | ||
| [[autodoc]] LDMTextToImagePipeline |
There was a problem hiding this comment.
This is not related to this PR is it? Fine for me either way :)
|
(Note, the |
Can't find the email addresses now so just an in-official co-authoring 😅 But think that's ok given that none of the PRs were finished |
* Better error message for transformers dummy * [PIL] Better deprecation functionality * up
Should solve the deprecation problem with PIL:
Co-authored-by: @michellemoorre
Co-authored-by: @kashif
Co-authored-by: @allo-
Co-authored-by: @Thomas-MMJ