Preserve original image extension in backend processing #6456
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-builds.s3.amazonaws.com/3347301721498a4279402eae73d8f6ee2f3d9c1b/gradio-4.3.0-py3-none-any.whlInstall Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@3347301721498a4279402eae73d8f6ee2f3d9c1b#subdirectory=client/python" |
🦄 change detectedThis Pull Request includes changes to the following packages.
With the following changelog entry.
Maintainers or the PR author can modify the PR title to modify this entry.
|
freddyaboulton
requested review from
pngwn,
dawoodkhan82,
abidlabs,
aliabid94,
aliabd and
hannahblair
| @@ -150,8 +150,12 @@ def preprocess( | |||
| if payload.orig_name: | |||
| p = Path(payload.orig_name) | |||
| name = p.stem | |||
| suffix = p.suffix.replace(".", "") | |||
| if suffix in ["jpg", "jpeg"]: | |||
There was a problem hiding this comment.
weird that PIL doesn't recognize "jpg" as a valid format
gradio/image_utils.py
Outdated
| @@ -27,7 +28,7 @@ def format_image( | |||
| return np.array(im) | |||
| elif type == "filepath": | |||
| path = processing_utils.save_pil_to_cache( | |||
| im, cache_dir=cache_dir, name=name, format=fmt or "png" # type: ignore | |||
| im, cache_dir=cache_dir, name=name, format=fmt or format # type: ignore | |||
There was a problem hiding this comment.
The only risk (admittedly small) I can see here is that there are some image formats that PIL can read but not save as: https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#read-only-formats
They all look very rare to me, but perhaps a more robust approach would be to try / catch this, and save as "png" as a fallback
There was a problem hiding this comment.
Yea that's a good point
| @@ -21,7 +21,7 @@ test("Image click-to-upload uploads image successfuly. Clear button dispatches e | |||
| await page.getByLabel("Download").click(); | |||
| const download = await downloadPromise; | |||
| // Automatically convert to png in the backend since PIL is very picky | |||
| await expect(download.suggestedFilename()).toBe("cheetah1.png"); | |||
| await expect(download.suggestedFilename()).toBe("cheetah1.jpeg"); | |||
|
Awesome thanks for taking care of this @freddyaboulton. Left a small note, otherwise lgtm |
gradio/image_utils.py
Outdated
| @@ -39,10 +40,12 @@ def format_image( | |||
|
|
|||
|
|
|||
| def save_image(y: np.ndarray | _Image.Image | str | Path, cache_dir: str): | |||
| # numpy and PIL get saved to png as default format | |||
There was a problem hiding this comment.
Isn't this going to account for like 90% of cases? Does opening an image with PIL or numpy not retain any meta information about the file that we can use to recreate the format?
There was a problem hiding this comment.
I don't think we can do better for numpy as there's no natural encoding choice. But we can try to read the format property for PIL.
|
Nice thanks @freddyaboulton! I suggested a more specific exception, otherwise lgtm |
Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
freddyaboulton
deleted the
preserve-file-extension-image-preprocessing
branch
Description
Jpegs were being converted to png always.
e2e test reflects this change.
🎯 PRs Should Target Issues
Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.
Not adhering to this guideline will result in the PR being closed.
Tests
PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests:
bash scripts/run_all_tests.shYou may need to run the linters:
bash scripts/format_backend.shandbash scripts/format_frontend.sh