Fix the download button of the `gr.Gallery()` component to work #6487

whitphx · 2023-11-19T09:03:39Z

Description

Fix the bug of the download button on gr.Gallery().

Closes: #6486

gradio-pr-bot · 2023-11-19T09:03:54Z

🪼 branch checks and previews

•	Name	Status	URL
	Spaces	ready!	Spaces preview
	Website	ready!	Website preview
	Storybook	ready!	Storybook preview
	Visual tests	all good!	Build review
🦄	Changes	detected!	Details

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/58456569f2c99c23c99cc5121cecfea11d514927/gradio-4.8.0-py3-none-any.whl

Install Gradio Python Client from this PR

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

gradio-pr-bot · 2023-11-19T09:04:29Z

🦄 change detected

This Pull Request includes changes to the following packages.

Package	Version
`@gradio/gallery`	`patch`
`gradio`	`patch`

Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Fix the download button of the gr.Gallery() component to work

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.

whitphx · 2023-11-19T09:22:58Z

js/gallery/shared/Gallery.svelte

+								if (selected_image == null) {
+									return;
+								}
+								const { url, orig_name } = selected_image;


With the current server-side code, orig_name is never set and the downloaded file name is always "image".
@abidlabs ~~Should Python-side be modified to set orig_name as well?~~ I modified gr.Gallery() so (779aa2c). Plz let me know if it shoudl be reverted.

That's a good fix @whitphx - thanks

hannahblair · 2023-11-20T16:21:51Z

Hey @whitphx! ~~I've just tested this and the images are downloading as html files 🤔~~

Ignore the above - I needed to rebuild the FE. So I'm still experiencing some weird behaviour when running the gallery component demo. See this recording:

To summarise the above:

The first image downloaded the image as a .jpg, as expected.
The second, third, and fourth images resulted in a CORS error
The final image tried to download this url:

gradio/components/gallery.py

freddyaboulton

Thanks for the fix! Do you mind modifying the 2e2 test (gallery_component_events.spec.ts) to download a file from the gallery and make sure it has the right name?

The video tests show how to download files with playwright, should be something like

	const downloadPromise = page.waitForEvent("download");
	await page.getByLabel("Download").click();
	const download = await downloadPromise;
	await expect(download.suggestedFilename()).toBe(<name>);

whitphx · 2023-11-21T05:01:49Z

@hannahblair @freddyaboulton Thank you for the review!

I modified the code

restoring the original <a>-tag-based download link which is like other places.
instead fixing the Python code of gr.Gallery() so its image URL resolution works like gr.Image and the same-origin URL is always passed to the frontend.

Then the CORS problem should be solved and updating the E2E test became unnecessary.
Please take a look again, thanks.

freddyaboulton · 2023-11-21T17:40:54Z

gradio/components/gallery.py

            else:
                raise ValueError(f"Cannot process type as image: {type(img)}")
            entry = GalleryImage(
-                image=FileData(path=file_path, url=url), caption=caption
+                # Leave `url` as None so it will be replaced with a local cache path during postprocessing (https://github.com/gradio-app/gradio/blob/c5ea13bdf7363224863760e53fbcdbed81754628/gradio/blocks.py#L1421)


This will introduce a regression in performance when compared to main.

You can check with the gallery_component_events demo. The postprocess will take several seconds as opposed to < 1 second.

This change

Main

In the past, people have used gr.Gallery to store many image URLs so I'm worried about degrading the experience for them.

That's an important point, thanks.
I understand gr.Gallery() is not just a group of gr.Images() but the quantity matters.
So I'm gonna go back to the previous path and improve it.

This reverts commit 4e2aa3f.

This reverts commit 007caa2.

gradio/components/gallery.py

abidlabs · 2023-12-09T00:21:54Z

Awesome PR, thanks for this nice bug fix @whitphx! Left a couple of very minor points above

Co-authored-by: Abubakar Abid <abubakar@huggingface.co>

This reverts commit 4d6e127.

This reverts commit f2bfad0.

This reverts commit 69ab93c.

whitphx · 2023-12-09T16:57:08Z

The suggested type syntax didn't work.
I think this is because from __future__ import annotations postpones the evaluations of annotations only, but does nothing about other things such as variable assignments which is used for type aliasing.

Ref:

whitphx · 2023-12-09T16:58:16Z

The failed test is only the flaky one. Will merge this ignoring it.

whitphx force-pushed the fix-gallery-download-button branch 2 times, most recently from cd257c2 to 87e4b23 Compare November 19, 2023 09:22

whitphx commented Nov 19, 2023

View reviewed changes

whitphx requested review from abidlabs, pngwn and hannahblair November 19, 2023 09:23

whitphx force-pushed the fix-gallery-download-button branch 2 times, most recently from 8193a85 to 779aa2c Compare November 19, 2023 10:52

whitphx marked this pull request as ready for review November 19, 2023 14:29

freddyaboulton reviewed Nov 20, 2023

View reviewed changes

gradio/components/gallery.py Show resolved Hide resolved

freddyaboulton reviewed Nov 20, 2023

View reviewed changes

whitphx force-pushed the fix-gallery-download-button branch 5 times, most recently from e340c1f to e96d7dd Compare November 21, 2023 04:58

whitphx force-pushed the fix-gallery-download-button branch 2 times, most recently from 378f930 to dcac42b Compare November 21, 2023 05:05

whitphx requested a review from freddyaboulton November 21, 2023 05:14

whitphx force-pushed the fix-gallery-download-button branch 5 times, most recently from 9acc972 to f8fb748 Compare November 21, 2023 07:01

freddyaboulton reviewed Nov 21, 2023

View reviewed changes

whitphx force-pushed the fix-gallery-download-button branch from 849802f to f50238c Compare November 22, 2023 02:53

whitphx force-pushed the fix-gallery-download-button branch 2 times, most recently from e718b95 to d403fab Compare November 22, 2023 03:21

whitphx added 4 commits November 22, 2023 12:22

Revert "Fix Gallery's tests"

21251bd

This reverts commit 4e2aa3f.

Revert "Fix the frontend test for gallery"

7c7136f

This reverts commit 007caa2.

Fix for linter

b1d04e8

Add a test about the download button

93a173d

whitphx force-pushed the fix-gallery-download-button branch from 1ecac84 to 93a173d Compare November 22, 2023 03:22

whitphx added 2 commits November 22, 2023 13:34

Fix type defs on Gallery.postprocess

a282421

Improve TestGallery

6e556c3

whitphx force-pushed the fix-gallery-download-button branch from 4ab12bb to 6e556c3 Compare November 22, 2023 04:38

add changeset

3cd08ac

whitphx requested a review from freddyaboulton November 22, 2023 04:56

whitphx and others added 3 commits December 5, 2023 09:59

Merge branch 'main' into fix-gallery-download-button

ee3f125

Merge branch 'main' into fix-gallery-download-button

5c802ee

Merge branch 'main' into fix-gallery-download-button

3403e48

abidlabs reviewed Dec 9, 2023

View reviewed changes

gradio/components/gallery.py Show resolved Hide resolved

abidlabs reviewed Dec 9, 2023

View reviewed changes

gradio/components/gallery.py Show resolved Hide resolved

abidlabs approved these changes Dec 9, 2023

View reviewed changes

whitphx and others added 6 commits December 9, 2023 13:57

Update gradio/components/gallery.py

f2bfad0

Co-authored-by: Abubakar Abid <abubakar@huggingface.co>

Update gradio/components/gallery.py

4d6e127

Co-authored-by: Abubakar Abid <abubakar@huggingface.co>

Revert "Update gradio/components/gallery.py"

2bc9b00

This reverts commit 4d6e127.

Revert "Update gradio/components/gallery.py"

0c1cd2d

This reverts commit f2bfad0.

Use tuple instead of typing.Tuple

69ab93c

Revert "Use tuple instead of typing.Tuple"

154817b

This reverts commit 69ab93c.

whitphx merged commit 9a5811d into main Dec 9, 2023
14 of 15 checks passed

whitphx deleted the fix-gallery-download-button branch December 9, 2023 16:58

pngwn mentioned this pull request Dec 9, 2023

chore: update versions #6670

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the download button of the `gr.Gallery()` component to work #6487

Fix the download button of the `gr.Gallery()` component to work #6487

whitphx commented Nov 19, 2023

gradio-pr-bot commented Nov 19, 2023 •

edited

gradio-pr-bot commented Nov 19, 2023 •

edited

Something isn't right?

whitphx Nov 19, 2023 •

edited

freddyaboulton Nov 20, 2023

hannahblair commented Nov 20, 2023 •

edited

freddyaboulton left a comment

whitphx commented Nov 21, 2023 •

edited

freddyaboulton Nov 21, 2023

whitphx Nov 22, 2023

abidlabs commented Dec 9, 2023 •

edited

whitphx commented Dec 9, 2023

whitphx commented Dec 9, 2023

Fix the download button of the gr.Gallery() component to work #6487

Fix the download button of the gr.Gallery() component to work #6487

Conversation

whitphx commented Nov 19, 2023

Description

gradio-pr-bot commented Nov 19, 2023 • edited

🪼 branch checks and previews

gradio-pr-bot commented Nov 19, 2023 • edited

🦄 change detected

This Pull Request includes changes to the following packages.

With the following changelog entry.

Something isn't right?

whitphx Nov 19, 2023 • edited

Choose a reason for hiding this comment

freddyaboulton Nov 20, 2023

Choose a reason for hiding this comment

hannahblair commented Nov 20, 2023 • edited

freddyaboulton left a comment

Choose a reason for hiding this comment

whitphx commented Nov 21, 2023 • edited

freddyaboulton Nov 21, 2023

Choose a reason for hiding this comment

This change

Main

whitphx Nov 22, 2023

Choose a reason for hiding this comment

abidlabs commented Dec 9, 2023 • edited

whitphx commented Dec 9, 2023

whitphx commented Dec 9, 2023

Fix the download button of the `gr.Gallery()` component to work #6487

Fix the download button of the `gr.Gallery()` component to work #6487

gradio-pr-bot commented Nov 19, 2023 •

edited

gradio-pr-bot commented Nov 19, 2023 •

edited

whitphx Nov 19, 2023 •

edited

hannahblair commented Nov 20, 2023 •

edited

whitphx commented Nov 21, 2023 •

edited

abidlabs commented Dec 9, 2023 •

edited