From 86944abbabad62e53e644bd7375b9a56d66c1675 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 15 Jan 2022 16:08:37 +1100 Subject: [PATCH 1/2] Deprecated show_file "file" argument in favour of "path" --- Tests/test_imageshow.py | 15 +++++++++++ src/PIL/ImageShow.py | 59 +++++++++++++++++++++++++++++++---------- 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/Tests/test_imageshow.py b/Tests/test_imageshow.py index 5981e22c0..02edfdfa1 100644 --- a/Tests/test_imageshow.py +++ b/Tests/test_imageshow.py @@ -79,3 +79,18 @@ def test_ipythonviewer(): im = hopper() assert test_viewer.show(im) == 1 + + +@pytest.mark.skipif( + not on_ci() or is_win32(), + reason="Only run on CIs; hangs on Windows CIs", +) +def test_file_deprecated(): + for viewer in ImageShow._viewers: + with pytest.warns(DeprecationWarning): + try: + viewer.show_file(file="test.jpg") + except NotImplementedError: + pass + with pytest.raises(TypeError): + viewer.show_file() diff --git a/src/PIL/ImageShow.py b/src/PIL/ImageShow.py index 2135293e5..068b7f784 100644 --- a/src/PIL/ImageShow.py +++ b/src/PIL/ImageShow.py @@ -16,6 +16,7 @@ import shutil import subprocess import sys import tempfile +import warnings from shlex import quote from PIL import Image @@ -105,9 +106,19 @@ class Viewer: """Display the given image.""" return self.show_file(self.save_image(image), **options) - def show_file(self, file, **options): - """Display the given file.""" - os.system(self.get_command(file, **options)) + def show_file(self, path=None, **options): + """Display given file.""" + 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'") + os.system(self.get_command(path, **options)) return 1 @@ -145,18 +156,28 @@ class MacViewer(Viewer): command = f"({command} {quote(file)}; sleep 20; rm -f {quote(file)})&" return command - def show_file(self, file, **options): + def show_file(self, path=None, **options): """Display given file""" - fd, path = tempfile.mkstemp() + 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'") + fd, temp_path = tempfile.mkstemp() with os.fdopen(fd, "w") as f: - f.write(file) - with open(path) as f: + f.write(path) + with open(temp_path) as f: subprocess.Popen( ["im=$(cat); open -a Preview.app $im; sleep 20; rm -f $im"], shell=True, stdin=f, ) - os.remove(path) + os.remove(temp_path) return 1 @@ -172,17 +193,27 @@ class UnixViewer(Viewer): command = self.get_command_ex(file, **options)[0] return f"({command} {quote(file)}; rm -f {quote(file)})&" - def show_file(self, file, **options): + def show_file(self, path=None, **options): """Display given file""" - fd, path = tempfile.mkstemp() + 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'") + fd, temp_path = tempfile.mkstemp() with os.fdopen(fd, "w") as f: - f.write(file) - with open(path) as f: - command = self.get_command_ex(file, **options)[0] + f.write(path) + with open(temp_path) as f: + command = self.get_command_ex(path, **options)[0] subprocess.Popen( ["im=$(cat);" + command + " $im; rm -f $im"], shell=True, stdin=f ) - os.remove(path) + os.remove(temp_path) return 1 From 5df83a57ff0f8362fc1d66180f777d5d79672342 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sun, 16 Jan 2022 11:38:34 +1100 Subject: [PATCH 2/2] Documented deprecation --- docs/deprecations.rst | 12 ++++++++++++ src/PIL/ImageShow.py | 21 ++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/deprecations.rst b/docs/deprecations.rst index ce30fdf3b..dbe65dda1 100644 --- a/docs/deprecations.rst +++ b/docs/deprecations.rst @@ -53,6 +53,18 @@ Before Pillow 8.3.0, ``ImagePalette`` required palette data of particular length default, and the size parameter could be used to override that. Pillow 8.3.0 removed the default required length, also removing the need for the size parameter. +ImageShow.Viewer.show_file file argument +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. deprecated:: 9.1.0 + +The ``file`` argument in :py:meth:`~PIL.ImageShow.Viewer.show_file()` has been +deprecated, replaced by ``path``. + +In effect, ``viewer.show_file("test.jpg")`` will continue to work unchanged. +``viewer.show_file(file="test.jpg")`` will raise a deprecation warning, and suggest +``viewer.show_file(path="test.jpg")`` instead. + Removed features ---------------- diff --git a/src/PIL/ImageShow.py b/src/PIL/ImageShow.py index 068b7f784..e437bbade 100644 --- a/src/PIL/ImageShow.py +++ b/src/PIL/ImageShow.py @@ -107,7 +107,12 @@ class Viewer: return self.show_file(self.save_image(image), **options) def show_file(self, path=None, **options): - """Display given file.""" + """ + 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( @@ -157,7 +162,12 @@ class MacViewer(Viewer): return command def show_file(self, path=None, **options): - """Display given file""" + """ + 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( @@ -194,7 +204,12 @@ class UnixViewer(Viewer): return f"({command} {quote(file)}; rm -f {quote(file)})&" def show_file(self, path=None, **options): - """Display given file""" + """ + 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(