UnixViewers modification: src/PIL/ImageShow.py, Tests/test_imageshow.py

- In the current implementation of UnixViewers threads were used in order to control opening/closing of images by external Linux/Unix viewers.
- The Python program remains opened till the user closes the image in the viewer.
- I used viewer thread and monitoring thread, the monitoring thread is responsible for removal of the temporary images after user closed the image in the viewer.
- Corresponding documentation is updated.
- Tests are updated: Tests/test_imageshow.py, Tests/helper.py

Tests/helper.py

@@ -27,8 +27,7 @@ if os.environ.get("SHOW_ERRORS", None):
     class test_image_results:
         @staticmethod
         def upload(a, b):
-            a.show()
+            return None
-            b.show()
 
 elif "GITHUB_ACTIONS" in os.environ:
     HAS_UPLOADER = True
@@ -316,6 +315,10 @@ def is_win32():
     return sys.platform.startswith("win32")
 
 
+def is_macos():
+    return sys.platform.startswith("darwin")
+
+
 def is_pypy():
     return hasattr(sys, "pypy_translation_info")
 

Tests/test_imageshow.py

@@ -2,7 +2,7 @@ import pytest
 
 from PIL import Image, ImageShow
 
-from .helper import hopper, is_win32, on_ci
+from .helper import hopper, is_macos, is_win32, on_ci
 
 
 def test_sanity():
@@ -41,9 +41,11 @@ def test_viewer_show(order):
     ImageShow._viewers.pop(0)
 
 
-@pytest.mark.skipif(
+@pytest.mark.skip(
-    not on_ci() or is_win32(),
+    reason="""Current implementation of some viewers requires manual closing of an image,
-    reason="Only run on CIs; hangs on Windows CIs",
+        because of that the tests calling show() method will hang infinitely.
+        Please also note that this test duplicates test_viewer_show() test.
+    """
 )
 def test_show():
     for mode in ("1", "I;16", "LA", "RGB", "RGBA"):
@@ -63,9 +65,33 @@ def test_viewer():
 def test_viewers():
     for viewer in ImageShow._viewers:
         try:
-            viewer.get_command("test.jpg")
+            cmd = viewer.get_command("test.jpg")
+            assert isinstance(cmd, str)
         except NotImplementedError:
-            pass
+            assert isinstance(viewer, ImageShow.IPythonViewer)
+
+
+@pytest.mark.skipif(
+    is_win32() or is_macos(), reason="The method is implemented for UnixViewers only"
+)
+def test_get_command_ex_interface():
+    """get_command_ex() method used by UnixViewers only"""
+
+    file = "some_image.jpg"
+    assert isinstance(file, str)
+
+    for viewer in ImageShow._viewers:
+        if isinstance(viewer, ImageShow.UnixViewer):
+            # method returns tuple
+            result = viewer.get_command_ex(file)
+            assert isinstance(result, tuple)
+            # file name is a required argument
+            with pytest.raises(TypeError) as err:
+                viewer.get_command_ex()
+            assert (
+                "get_command_ex() missing 1 required positional argument: 'file'"
+                in str(err.value)
+            )
 
 
 def test_ipythonviewer():
@@ -89,10 +115,12 @@ def test_file_deprecated(tmp_path):
     f = str(tmp_path / "temp.jpg")
     for viewer in ImageShow._viewers:
         hopper().save(f)
-        with pytest.warns(DeprecationWarning):
+        if not isinstance(viewer, ImageShow.UnixViewer):
-            try:
+            # do not run this assertion with UnixViewers due to implementation
-                viewer.show_file(file=f)
+            with pytest.warns(DeprecationWarning):
-            except NotImplementedError:
+                try:
-                pass
+                    viewer.show_file(file=f)
+                except NotImplementedError:
+                    pass
         with pytest.raises(TypeError):
             viewer.show_file()

src/PIL/ImageShow.py

@@ -15,6 +15,7 @@ import os
 import shutil
 import subprocess
 import sys
+import threading
 import warnings
 from shlex import quote
 
@@ -48,9 +49,10 @@ def show(image, title=None, **options):
     Display a given image.
 
     :param image: An image object.
-    :param title: Optional title. Not all viewers can display the title.
+    :param title: sets string displayed in image window's titlebar;
+         ``title`` is optional: not all viewers support this option.
     :param \**options: Additional viewer options.
-    :returns: ``True`` if a suitable viewer was found, ``False`` otherwise.
+    :returns: 1 if a suitable viewer was found, 0 otherwise.
     """
     for viewer in _viewers:
         if viewer.show(image, title=title, **options):
@@ -193,19 +195,27 @@ if sys.platform == "darwin":
 
 
 class UnixViewer(Viewer):
+    opened_images = []
+
     def get_command(self, file, **options):
-        command = self.get_command_ex(file, **options)[0]
+        return self.get_command_ex(file, **options)[0]
-        return f"({command} {quote(file)}; rm -f {quote(file)})&"
 
-
+    def thread_monitor(self, th):
-class XDGViewer(UnixViewer):
+        """
-    """
+        Monitors image viewing threads. The last remaining monitoring thread
-    The freedesktop.org ``xdg-open`` command.
+        is responsible for removal of temporary images.
-    """
+        """
-
+        th.join()
-    def get_command_ex(self, file, **options):
+        if threading.active_count() == 2:
-        command = executable = "xdg-open"
+            for f in self.opened_images:
-        return command, executable
+                if os.path.isfile(f):
+                    try:
+                        os.remove(f)
+                    except OSError as e:
+                        print(f"failed to delete the file: {f}")
+                        print(e)
+            return 1
+        return 0
 
     def show_file(self, path=None, **options):
         """
@@ -225,150 +235,80 @@ class XDGViewer(UnixViewer):
                 path = options.pop("file")
             else:
                 raise TypeError("Missing required argument: 'path'")
-        subprocess.Popen(["xdg-open", path])
+
-        self._remove_path_after_delay(path)
+        path = quote(path)
+        command = self.get_command(path, **options)
+
+        kwargs = {
+            "stdout": subprocess.PIPE,
+        }
+
+        th = threading.Thread(
+            target=subprocess.run, args=(command.split(),), kwargs=kwargs, name=path
+        )
+        self.opened_images.append(th.name)
+        th.start()
+        th_monitor = threading.Thread(target=self.thread_monitor, args=(th,))
+        th_monitor.start()
         return 1
 
 
+class XDGViewer(UnixViewer):
+    """
+    The freedesktop.org ``xdg-open`` command.
+    """
+
+    def get_command_ex(self, file, **options):
+        executable = "xdg-open"
+        command = f"xdg-open {quote(file)}"
+        return command, executable
+
+
 class DisplayViewer(UnixViewer):
     """
-    The ImageMagick ``display`` command.
+    The ImageMagick ``display`` command. This viewer supports the ``title`` parameter.
-    This viewer supports the ``title`` parameter.
     """
 
     def get_command_ex(self, file, title=None, **options):
-        command = executable = "display"
+        executable = "display"
+        command = f"display {quote(file)}"
         if title:
-            command += f" -name {quote(title)}"
+            command = f"display -title {quote(title)} {quote(file)}"
         return command, executable
 
-    def show_file(self, path=None, **options):
-        """
-        Display given file.
-
-        Before Pillow 9.1.0, the first argument was ``file``. This is now deprecated,
-        and ``path`` should be used instead.
-        """
-        if path is None:
-            if "file" in options:
-                warnings.warn(
-                    "The 'file' argument is deprecated and will be removed in Pillow "
-                    "10 (2023-07-01). Use 'path' instead.",
-                    DeprecationWarning,
-                )
-                path = options.pop("file")
-            else:
-                raise TypeError("Missing required argument: 'path'")
-        args = ["display"]
-        if "title" in options:
-            args += ["-name", options["title"]]
-        args.append(path)
-
-        subprocess.Popen(args)
-        os.remove(path)
-        return 1
-
 
 class GmDisplayViewer(UnixViewer):
     """The GraphicsMagick ``gm display`` command."""
 
     def get_command_ex(self, file, **options):
         executable = "gm"
-        command = "gm display"
+        command = f"gm display {quote(file)}"
         return command, executable
 
-    def show_file(self, path=None, **options):
-        """
-        Display given file.
-
-        Before Pillow 9.1.0, the first argument was ``file``. This is now deprecated,
-        and ``path`` should be used instead.
-        """
-        if path is None:
-            if "file" in options:
-                warnings.warn(
-                    "The 'file' argument is deprecated and will be removed in Pillow "
-                    "10 (2023-07-01). Use 'path' instead.",
-                    DeprecationWarning,
-                )
-                path = options.pop("file")
-            else:
-                raise TypeError("Missing required argument: 'path'")
-        subprocess.Popen(["gm", "display", path])
-        os.remove(path)
-        return 1
-
 
 class EogViewer(UnixViewer):
     """The GNOME Image Viewer ``eog`` command."""
 
     def get_command_ex(self, file, **options):
         executable = "eog"
-        command = "eog -n"
+        command = f"eog -n {quote(file)}"
         return command, executable
 
-    def show_file(self, path=None, **options):
-        """
-        Display given file.
-
-        Before Pillow 9.1.0, the first argument was ``file``. This is now deprecated,
-        and ``path`` should be used instead.
-        """
-        if path is None:
-            if "file" in options:
-                warnings.warn(
-                    "The 'file' argument is deprecated and will be removed in Pillow "
-                    "10 (2023-07-01). Use 'path' instead.",
-                    DeprecationWarning,
-                )
-                path = options.pop("file")
-            else:
-                raise TypeError("Missing required argument: 'path'")
-        subprocess.Popen(["eog", "-n", path])
-        os.remove(path)
-        return 1
-
 
 class XVViewer(UnixViewer):
     """
-    The X Viewer ``xv`` command.
+    The X Viewer ``xv`` command. This viewer supports the ``title`` parameter.
-    This viewer supports the ``title`` parameter.
     """
 
     def get_command_ex(self, file, title=None, **options):
         # note: xv is pretty outdated.  most modern systems have
         # imagemagick's display command instead.
-        command = executable = "xv"
+        executable = "xv"
+        command = f"xv {quote(file)}"
         if title:
-            command += f" -name {quote(title)}"
+            command = f"xv -name {quote(title)} {quote(file)}"
         return command, executable
 
-    def show_file(self, path=None, **options):
-        """
-        Display given file.
-
-        Before Pillow 9.1.0, the first argument was ``file``. This is now deprecated,
-        and ``path`` should be used instead.
-        """
-        if path is None:
-            if "file" in options:
-                warnings.warn(
-                    "The 'file' argument is deprecated and will be removed in Pillow "
-                    "10 (2023-07-01). Use 'path' instead.",
-                    DeprecationWarning,
-                )
-                path = options.pop("file")
-            else:
-                raise TypeError("Missing required argument: 'path'")
-        args = ["xv"]
-        if "title" in options:
-            args += ["-name", options["title"]]
-        args.append(path)
-
-        subprocess.Popen(args)
-        os.remove(path)
-        return 1
-
 
 if sys.platform not in ("win32", "darwin"):  # unixoids
     if shutil.which("xdg-open"):