XDGViewer and GmDisplayViewer fail to show images

[alv2017]

What did you do?

1. I created script to check Unix image viewers available in PIL.ImageShow module. The script example for XDGViewer is provided
   below.

2. I run the script for all Linux/Unix viewers except XVViewer (I didn't manage to obtain it)

What did you expect to happen?

I expected to open my image with each single viewer I tried to use:
XDGViewer, DisplayViewer, GmDisplayViewer, EogViewer

What actually happened?

The image failed to open with

a) XDGViewer
b) GmDisplayViewer

What are your OS, Python and Pillow versions?

• OS: Ubuntu, 20.04.3 LTS (Focal Fossa)
• Python: 3.9.9
• Pillow: 9.1.0.dev0

from PIL import Image
from PIL.ImageShow import XDGViewer

if __name__ == "__main__":
    viewer = XDGViewer()
    img_file = "path/to/image_file"
    with Image.open(img_file) as im:
        viewer.show(im)

[radarhere]

When you use GmDisplayViewer, what error do you see?

[alv2017]

In case of GmDisplayViewer the image is not showing up, without reporting anything.
I suspect that the image gets closed before the user sees it.

[radarhere]

If you modify your Pillow installation to include sleep 20 before removing the image file - radarhere@3eae2ef - does that fix both XDGViewer and GmDisplayViewer?

[alv2017]

I think that I might have a better solution. :)
(adding sleep 20 fixes the problem)

[alv2017]

I also suggest to have a look at Open multiple images case. This doesn't work as expected as well: try sliding through the images in a viewer.

import os
from PIL import Image


if __name__ == "__main__":
    img_dir = "path/to/img/directory"
    img_files = ["file1.png", "file2.png", "file3.png"]

    for img_file in img_files:
        img_location = os.path.join(img_dir, img_file)
        with Image.open(img_location) as im:
            im.show(im)

[radarhere]

Just for the record - im.show(im) is incorrect. im.show() is I think what you're after, rather than trying to supply im as the title argument.

Could you expand on what problem you're seeing when showing multiple images? After inserting sleep 20 into a Pillow 9.0 installation, I don't see anything wrong.

imageshow.mov

[alv2017]

Using either im.show() or im.show(im) results in the same problem: after 20 s images disappear :)
(Try sliding over the images, let's say, after 30s)

[alv2017]

I believe that the perfect viewer function

a) allows to open one image or multiple images in one script;

b) multiple images can be viewed simultaneously, and user can switch from one image to another when viewing the images;

c) image is closed when the user closes it in the viewer;

d) temporary image files are deleted from the user machine.

[BrettRyland]

This also occurs with gwenview in KDE on KUbuntu versions as I posted here.
In addition to your perfect viewer function requirements, I would add that the viewer should be lightweight and with a clean interface. In my opinion, ImageMagick (via display instead of xdg-open) worked well for this.

[alv2017]

Actually I fixed all the viewers, and I made a DisplayViewer (which is ImageMagick) the default viewer. I will make a PR today or tomorrow.

[BrettRyland]

OK, sounds good. Thanks!

[radarhere]

I would add that the viewer should be lightweight and with a clean interface. In my opinion, ImageMagick (via display instead of xdg-open) worked well for this.

The user behind #5897 clearly had other thoughts, and so does a user in #5945 (comment). If there's not a solution that's going to make everyone happy, I'm inclined to leave the priority of xdg-open as it is.

[BrettRyland]

This is a matter of system configuration of the xdg-open and display alternatives. The former is for general purpose opening of a file (in this case an image), which may include full-fledged editing software (e.g., Krita or Gimp) or a gallery-like viewer (e.g., gwenview), whereas the latter is for simply viewing the file.
I would argue that the purpose of the image viewer in Pillow is simply for displaying an image (which may lie purely in memory) and is therefore more appropriately served by display. However, as I mentioned in the other issue, a proper solution to this is to allow the user to configure which display program to use via a configuration file and only fall back on the system alternatives if no such configuration exists.

[alv2017]

I also have some thoughts regarding the viewers:

1. the viewers are different, they behave differently, and it makes difficult to develop a consistent solution;
2. xdg-open is not a viewer itself, it is the command that passes the image to the actual image viewer, and it
   makes capturing the process handled by xdg-open more difficult. Not to mention that behind the scene we have some
   unknown viewer with its own philosophy of image handling. That is one of the reasons of adding xdg-open as the last resort option.
3. display viewer by ImageMagic looks quite promissing, because it can handle not only files but the byte streams as well,
   so effectively there is no need to create a temporary file, there is no need and removing the images after they have been shown.

To summarize: the problem with UnixViewers is lack of consistency in the approach of how the things should work. The key questions to answer: 1) what kind of viewers to support? 2) how the viewing process needs to be handled?

[radarhere]

#6045 has now been merged, so in the next release of Pillow, images will now longer be remove on Unix.
@alv2017 approves of closing this.

@BrettRyland regarding your request for the image viewer to be controlled from a configuration file on disk, Pillow doesn't use that technique for anything else at the moment. A similar idea is being discussed in #6070, but there is reluctance to move control of what is happening outside of the code.

[BrettRyland]

@BrettRyland regarding your request for the image viewer to be controlled from a configuration file on disk, Pillow doesn't use that technique for anything else at the moment. A similar idea is being discussed in #6070, but there is reluctance to move control of what is happening outside of the code.

Fair enough. I understand the reluctance to implement a configuration file if that approach isn't being used anywhere else. I would suggest allowing the user to specify which viewer to use for the session in a simple single-line command that can be run early in a script or session (or put into ~/.ipython/profile_default/ipython_config.py, for example) then instead. You could even include an option in that command to automatically delete the file after a delay if the user desires to prevent build-up of temporary files, since that's now been disabled.

[nulano]

I would suggest allowing the user to specify which viewer to use for the session in a simple single-line command that can be run early in a script or session (or put into ~/.ipython/profile_default/ipython_config.py, for example) then instead.

Untested, but I believe the following example should do that:

from PIL import ImageShow
from shlex import quote
class MyViewer(ImageShow.Viewer):
    def get_command(self, file, **options):
        return f"my-viewer {quote(file)}"
ImageShow.register(MyViewer(), order=0)

[BrettRyland]

That's not exactly a simple single-line command that the average user could be expected to use.
My typical usage of PIL is

from PIL import Image
# do some stuff to load or generate an image 'img'
img.show()

I would think something along the lines of calling

Image.set_show_options(command='display', tmp_file_removal_delay=3)

after importing Image would be more intuitive for the average user, with something like what you wrote being implemented behind the scenes. (The tmp_file_removal_delay could a delay in seconds or 0 for never, for example.)

[radarhere]

Just demonstrating some more alternatives that exist at present.

• Here is one line to specify the viewer.

from PIL import Image, ImageShow
with Image.open("test.png") as im:
    ImageShow.DisplayViewer().show(im)

• Here is one line that will change what im.show() uses for the rest of the script.

from PIL import Image, ImageShow
ImageShow._viewers = [ImageShow.DisplayViewer()]
with Image.open("test.png") as im:
    im.show()

[radarhere]

Note that tmp_file_removal_delay should not be necessary on Unix after #6045

Here's another alternative.

https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.show

This method calls PIL.ImageShow.show() internally. You can use PIL.ImageShow.register() to override its default behaviour.

https://pillow.readthedocs.io/en/stable/reference/ImageShow.html#PIL.ImageShow.register

The register() function is used to register additional viewers.

Parameters:
viewer – The viewer to be registered.
order – Zero or a negative integer to prepend this viewer to the list, a positive integer to append it.

So, the user can do the following.

from PIL import Image, ImageShow
ImageShow.register(ImageShow.DisplayViewer(), 0)
with Image.open("test.png") as im:
    im.show()

Edit: I now realise @nulano pointed this out already. I'm adding to that answer by demonstrating that it is documented, and that you don't need to create your own viewer if you just want to use one of the existing ones.

That would seem to be a solution that is both simple and documented. Does that resolve this?

[BrettRyland]

#6045 sidesteps the issue of tmp file removal by simply not doing it, which can lead to a large build-up of tmp files if the system is rarely rebooted, though this is not really an issue on most systems unless the system in question is generating a lot of images this way. The point of tmp_file_removal_delay (or something similar) is that it allows the user to specify that the file should be removed and that the onus is then on the user to make sure that this doesn't affect the viewer being used.

The ImageShow.register(ImageShow.DisplayViewer(), 0) one-liner satisfies the simple condition, but it wasn't obvious to me in the documentation that this was the appropriate way to do so. Perhaps this could be added as an example to the PIL.Image.Image.show documentation after the "This method calls PIL.ImageShow.show() internally. You can use PIL.ImageShow.register() to override its default behaviour." line and/or in the PIL.ImageShow.register documentation?

[radarhere]

I've created PR #6078 to resolve this, by adding examples to the ImageShow.register() docstring, and noting under UnixViewer that temporary files are not removed.