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

Add format parameter to gr.Image, gr.Gallery, gr.AnnotatedImage, gr.Plot to control format to save image files in #7680

Merged
merged 13 commits into from Mar 19, 2024

Conversation

dfl
Copy link
Contributor

@dfl dfl commented Mar 12, 2024

Description

preserve file format during upload, default to PNG only when no other format is available.
adds format parameter to Image, AnnotatedImage, and Plot components.

Closes: #7486

Tests

I'm not able to get the test suite working properly on my mac. Having problems with dependencies.

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Mar 12, 2024

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
🦄 Changes detected! Details

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/41636f7ca124b44a596d9fa67e10c26f42982823/gradio-4.21.0-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@41636f7ca124b44a596d9fa67e10c26f42982823#subdirectory=client/python"

Copy link
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thanks @dfl! Overall looks great. Left some comments that we should address. And can you please add unit tests? Thanks

@@ -78,23 +84,31 @@ def get_pil_metadata(pil_image):

def encode_pil_to_bytes(pil_image, format="png"):
with BytesIO() as output_bytes:
pil_image.save(output_bytes, format, pnginfo=get_pil_metadata(pil_image))
if format == "png":
params = {"pnginfo": get_pil_metadata(pil_image)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can close #7657 then

@@ -51,13 +51,17 @@ def format_image(
)


def save_image(y: np.ndarray | PIL.Image.Image | str | Path, cache_dir: str):
def save_image(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The type hint for format is wrong (it can't be None)

)
base_img_path = str(utils.abspath(base_file))
elif isinstance(base_img, PIL.Image.Image):
base_file = processing_utils.save_pil_to_cache(
base_img, cache_dir=self.GRADIO_CACHE
base_img, cache_dir=self.GRADIO_CACHE, format=self.format
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to update the type hint of format in save_pil_to_cache to allow for an arbitrary string

@@ -51,13 +51,17 @@ def format_image(
)


def save_image(y: np.ndarray | PIL.Image.Image | str | Path, cache_dir: str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Right now format_image will give precedence for the current format of the image instead of the format argument. It should be the other way around.

@@ -199,7 +202,7 @@ def postprocess(
return None
if isinstance(value, str) and value.lower().endswith(".svg"):
return FileData(path=value, orig_name=Path(value).name)
saved = image_utils.save_image(value, self.GRADIO_CACHE)
saved = image_utils.save_image(value, self.GRADIO_CACHE, self.format or "png")
Copy link

Choose a reason for hiding this comment

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

Will this lead to loss of image quality for lossy formats?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose it depends on what the default JPG encoding settings are?

@freddyaboulton freddyaboulton self-assigned this Mar 18, 2024
@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Mar 18, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
gradio minor
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Add format parameter to gr.Image, gr.Gallery, gr.AnnotatedImage, gr.Plot to control format to save image files in

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@freddyaboulton freddyaboulton changed the title Preserve image format Add format parameter to gr.Image, gr.Gallery, gr.AnnotatedImage, gr.Plot to control format to save image files in Mar 19, 2024
@freddyaboulton
Copy link
Collaborator

This should be good for review now

@@ -56,6 +57,7 @@ def __init__(
"""
Parameters:
value: Optionally, supply a default plot object to display, must be a matplotlib, plotly, altair, or bokeh figure, or a callable. If callable, the function will be called whenever the app loads to set the initial value of the component.
format: File format to save matplotlib plots as, such as 'jpg' or 'png'. If set to None, will default to 'png'.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
format: File format to save matplotlib plots as, such as 'jpg' or 'png'. If set to None, will default to 'png'.
format: File format in which to send plot data to the frontend, such as 'jpg' or 'png'. If set to None, will default to 'png'.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This only applies to Matplotlib plots so I'll keep that part and make the other suggestions in wording

@@ -67,6 +68,7 @@ def __init__(
"""
Parameters:
value: Tuple of base image and list of (annotation, label) pairs.
format: Format used to save images, such as 'jpeg' or 'png'. The jpeg format uses less memory at the cost of image quality. If set to None, will use "png". This parameter only takes effect when numpy arrays or PIL Images are returned from the prediction function.
Copy link
Member

@abidlabs abidlabs Mar 19, 2024

Choose a reason for hiding this comment

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

Suggested change
format: Format used to save images, such as 'jpeg' or 'png'. The jpeg format uses less memory at the cost of image quality. If set to None, will use "png". This parameter only takes effect when numpy arrays or PIL Images are returned from the prediction function.
format: Format to save image before it is returned to the frontend, such as 'png' or 'jpeg'. This parameter only takes effect when the base image is returned from the prediction function as a numpy array or a PIL Image. The format should be supported by the PIL library.

@@ -49,6 +49,7 @@ def __init__(
]
| None = None,
*,
format: str | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
format: str | None = None,
format: str = "png",

@@ -141,12 +144,12 @@ def postprocess(
base_img = np.array(PIL.Image.open(base_img))
elif isinstance(base_img, np.ndarray):
base_file = processing_utils.save_img_array_to_cache(
base_img, cache_dir=self.GRADIO_CACHE
base_img, cache_dir=self.GRADIO_CACHE, format=self.format or "png"
Copy link
Member

Choose a reason for hiding this comment

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

If we make the change where format is, by default, png, we won't have to handle the case where it has to be None here and below

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea good point

@@ -76,6 +77,7 @@ def __init__(
"""
Parameters:
value: List of images to display in the gallery by default. If callable, the function will be called whenever the app loads to set the initial value of the component.
format: Format used to save images, such as 'jpeg' or 'png'. The jpeg format uses less memory at the cost of image quality. If set to None, will use "png". This parameter only takes effect when numpy arrays or PIL Images are returned from the prediction function.
Copy link
Member

@abidlabs abidlabs Mar 19, 2024

Choose a reason for hiding this comment

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

Suggested change
format: Format used to save images, such as 'jpeg' or 'png'. The jpeg format uses less memory at the cost of image quality. If set to None, will use "png". This parameter only takes effect when numpy arrays or PIL Images are returned from the prediction function.
format: Format to save images before they are returned to the frontend, such as 'png' or 'jpeg'. This parameter only applies to images that are returned from the prediction function as numpy arrays or PIL Images. The format should be supported by the PIL library.

@@ -50,6 +50,7 @@ def __init__(
| Callable
| None = None,
*,
format: str | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
format: str | None = None,
format: str = "png",

)
base_img_path = str(utils.abspath(base_file))
elif isinstance(base_img, PIL.Image.Image):
base_file = processing_utils.save_pil_to_cache(
base_img, cache_dir=self.GRADIO_CACHE
base_img, cache_dir=self.GRADIO_CACHE, format=self.format or "png"
Copy link
Member

Choose a reason for hiding this comment

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

Same

@@ -176,12 +179,12 @@ def _save(img):
img, caption = img
if isinstance(img, np.ndarray):
file = processing_utils.save_img_array_to_cache(
img, cache_dir=self.GRADIO_CACHE
img, cache_dir=self.GRADIO_CACHE, format=self.format or "png"
Copy link
Member

Choose a reason for hiding this comment

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

If we make the change where format is, by default, png, we won't have to handle the case where it has to be None here and below

)
file_path = str(utils.abspath(file))
elif isinstance(img, PIL.Image.Image):
file = processing_utils.save_pil_to_cache(
img, cache_dir=self.GRADIO_CACHE
img, cache_dir=self.GRADIO_CACHE, format=self.format or "png"
Copy link
Member

Choose a reason for hiding this comment

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

Same

@@ -43,6 +43,7 @@ def __init__(
self,
value: str | PIL.Image.Image | np.ndarray | None = None,
*,
format: str | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
format: str | None = None,
format: str = "png",

@@ -69,6 +70,7 @@ def __init__(
"""
Parameters:
value: A PIL Image, numpy array, path or URL for the default value that Image component is going to take. If callable, the function will be called whenever the app loads to set the initial value of the component.
format: Format used to save images, such as 'jpeg' or 'png'. The jpeg format uses less memory at the cost of image quality. This parameter has no effect on SVG files. If set to None, image will keep uploaded format.
Copy link
Member

@abidlabs abidlabs Mar 19, 2024

Choose a reason for hiding this comment

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

Suggested change
format: Format used to save images, such as 'jpeg' or 'png'. The jpeg format uses less memory at the cost of image quality. This parameter has no effect on SVG files. If set to None, image will keep uploaded format.
format: Format to save image, e.g. 'png' or 'jpeg'. The format should be supported by the PIL library. This parameter has no effect on SVG files or if the file is being returned to the frontend as a str filepath.

@abidlabs
Copy link
Member

abidlabs commented Mar 19, 2024

Just some nits to improve the docstrings, but otherwise lgtm @dfl and @freddyaboulton!

@freddyaboulton
Copy link
Collaborator

Thanks for the suggestions @abidlabs ! I made those modifications and merged in the latest main branch.

Copy link
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

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

Thanks @freddyaboulton! Let's get this in

@abidlabs abidlabs enabled auto-merge (squash) March 19, 2024 18:36
@abidlabs abidlabs merged commit 853d945 into gradio-app:main Mar 19, 2024
7 checks passed
@pngwn pngwn mentioned this pull request Mar 19, 2024
@freddyaboulton
Copy link
Collaborator

Thanks @dfl ! You'll be credited in the changelog!

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.

Reduce processed image display/downloading overhead
5 participants