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

Drop support for EOL Python 2.7 #4109

Merged
merged 28 commits into from
Nov 20, 2019
Merged

Drop support for EOL Python 2.7 #4109

merged 28 commits into from
Nov 20, 2019

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Oct 1, 2019

For #3642

Changes proposed in this pull request:

@hugovk hugovk added the Removal Removal of a feature, usually done in major releases label Oct 1, 2019
@hugovk hugovk added this to the 7.0.0 milestone Oct 1, 2019
.coveragerc Outdated Show resolved Hide resolved
Tests/test_file_libtiff.py Show resolved Hide resolved
winbuild/get_pythons.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
@jdufresne
Copy link
Contributor

The scripts in depends/ still make mention of EOL Pythons and install Python2 packages. Should they updated too? Just one example is depends/fedora_23.sh

# Installs all of the dependencies for Pillow for Fedora 23
# for both system Pythons 2.7 and 3.4

Both Python 3.4 and Fedora 23 are also EOL, so I'm not sure how often these are updated.

@hugovk
Copy link
Member Author

hugovk commented Oct 6, 2019

Yep, they've not been updated for a while and aren't directly used. Debian 8 is still supported but not for much longer. The others are all EOL (fedora_23.sh, freebsd_10.sh, ubuntu_12.04.sh, ubuntu_14.04.sh).

The installation docs say:

There are scripts to install the dependencies for some operating systems included in the depends directory. Also see the Dockerfiles in our docker images repo.

I guess we could remove the OS scripts from depends, and only point people to the Dockerfiles, which we use on the CI.

.travis.yml Outdated Show resolved Hide resolved
Tests/test_imagefont.py Outdated Show resolved Hide resolved
@jdufresne
Copy link
Contributor

I believe this will allow removing several intermediate bytearray objects.

$ python2 -c 'print(repr(bytes(3)))'
'3'
$ python2 -c 'print(repr(bytes(bytearray(3))))'
'\x00\x00\x00'
$ python3 -c 'print(repr(bytes(3)))'
b'\x00\x00\x00'
$ python3 -c 'print(repr(bytes(bytearray(3))))'
b'\x00\x00\x00'
diff --git a/Tests/test_file_gif.py b/Tests/test_file_gif.py
index 4ff9727e..677301bc 100644
--- a/Tests/test_file_gif.py
+++ b/Tests/test_file_gif.py
@@ -73,7 +73,7 @@ class TestFileGif(PillowTestCase):
             im = Image.frombytes(
                 "P",
                 (colors, colors),
-                bytes(bytearray(range(256 - colors, 256)) * colors),
+                bytes(range(256 - colors, 256)) * colors,
             )
             im = im.resize((size, size))
             outfile = BytesIO()
@@ -103,7 +103,7 @@ class TestFileGif(PillowTestCase):
         check(256, 511, 256)
 
     def test_optimize_full_l(self):
-        im = Image.frombytes("L", (16, 16), bytes(bytearray(range(256))))
+        im = Image.frombytes("L", (16, 16), bytes(range(256)))
         test_file = BytesIO()
         im.save(test_file, "GIF", optimize=True)
         self.assertEqual(im.mode, "L")
@@ -633,7 +633,7 @@ class TestFileGif(PillowTestCase):
         # that's > 128 items where the transparent color is actually
         # the top palette entry to trigger the bug.
 
-        data = bytes(bytearray(range(1, 254)))
+        data = bytes(range(1, 254))
         palette = ImagePalette.ImagePalette("RGB", list(range(256)) * 3)
 
         im = Image.new("L", (253, 1))
@@ -681,7 +681,7 @@ class TestFileGif(PillowTestCase):
 
         im = hopper("P")
         im_l = Image.frombytes("L", im.size, im.tobytes())
-        palette = bytes(bytearray(im.getpalette()))
+        palette = bytes(im.getpalette())
 
         out = self.tempfile("temp.gif")
         im_l.save(out, palette=palette)
@@ -696,7 +696,7 @@ class TestFileGif(PillowTestCase):
         # Forcing a non-straight grayscale palette.
 
         im = hopper("P")
-        palette = bytes(bytearray([255 - i // 3 for i in range(768)]))
+        palette = bytes([255 - i // 3 for i in range(768)])
 
         out = self.tempfile("temp.gif")
         im.save(out, palette=palette)
@@ -737,7 +737,7 @@ class TestFileGif(PillowTestCase):
         im.putpalette(ImagePalette.ImagePalette("RGB"))
         im.info = {"background": 0}
 
-        passed_palette = bytes(bytearray([255 - i // 3 for i in range(768)]))
+        passed_palette = bytes([255 - i // 3 for i in range(768)])
 
         GifImagePlugin._FORCE_OPTIMIZE = True
         try:
diff --git a/Tests/test_lib_pack.py b/Tests/test_lib_pack.py
index 6178184b..45958d2e 100644
--- a/Tests/test_lib_pack.py
+++ b/Tests/test_lib_pack.py
@@ -18,7 +18,7 @@ class TestLibPack(PillowTestCase):
 
         if isinstance(data, int):
             data_len = data * len(pixels)
-            data = bytes(bytearray(range(1, data_len + 1)))
+            data = bytes(range(1, data_len + 1))
 
         self.assertEqual(data, im.tobytes("raw", rawmode))
 
@@ -226,7 +226,7 @@ class TestLibUnpack(PillowTestCase):
         """
         if isinstance(data, int):
             data_len = data * len(pixels)
-            data = bytes(bytearray(range(1, data_len + 1)))
+            data = bytes(range(1, data_len + 1))
 
         im = Image.frombytes(mode, (len(pixels), 1), data, "raw", rawmode, 0, 1)
 
diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py
index b0d465fe..86e8264d 100644
--- a/src/PIL/TiffImagePlugin.py
+++ b/src/PIL/TiffImagePlugin.py
@@ -1772,7 +1772,7 @@ class AppendingTiffWriter:
         # pad to 16 byte boundary
         padBytes = 16 - pos % 16
         if 0 < padBytes < 16:
-            self.f.write(bytes(bytearray(padBytes)))
+            self.f.write(bytes(padBytes))
         self.offsetOfNewPage = self.f.tell()
 
     def setEndian(self, endian):

@hugovk
Copy link
Member Author

hugovk commented Oct 7, 2019

Thanks, removed redundant bytearrays in 4382413.

@jdufresne
Copy link
Contributor

Can now simplify some code using subprocess.DEVNULL:

https://docs.python.org/3/library/subprocess.html#subprocess.DEVNULL

diff --git a/Tests/helper.py b/Tests/helper.py
index 28248294..48fac0cd 100644
--- a/Tests/helper.py
+++ b/Tests/helper.py
@@ -325,11 +325,10 @@ def command_succeeds(cmd):
     """
     import subprocess
 
-    with open(os.devnull, "wb") as f:
-        try:
-            subprocess.call(cmd, stdout=f, stderr=subprocess.STDOUT)
-        except OSError:
-            return False
+    try:
+        subprocess.call(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.STDOUT)
+    except OSError:
+        return False
     return True
 
 
diff --git a/setup.py b/setup.py
index c72e0296..ff68b1e8 100755
--- a/setup.py
+++ b/setup.py
@@ -164,10 +164,10 @@ def _find_library_dirs_ldconfig():
         expr = r".* => (.*)"
         env = {}
 
-    null = open(os.devnull, "wb")
     try:
-        with null:
-            p = subprocess.Popen(args, stderr=null, stdout=subprocess.PIPE, env=env)
+        p = subprocess.Popen(
+            args, stderr=subprocess.DEVNULL, stdout=subprocess.PIPE, env=env
+        )
     except OSError:  # E.g. command not found
         return []
     [data, _] = p.communicate()
diff --git a/src/PIL/EpsImagePlugin.py b/src/PIL/EpsImagePlugin.py
index badeef4e..b7d5d7e5 100644
--- a/src/PIL/EpsImagePlugin.py
+++ b/src/PIL/EpsImagePlugin.py
@@ -57,8 +57,7 @@ def has_ghostscript():
         import subprocess
 
         try:
-            with open(os.devnull, "wb") as devnull:
-                subprocess.check_call(["gs", "--version"], stdout=devnull)
+            subprocess.check_call(["gs", "--version"], stdout=subprocess.DEVNULL)
             return True
         except OSError:
             # No Ghostscript
diff --git a/src/PIL/GifImagePlugin.py b/src/PIL/GifImagePlugin.py
index 69ff2f66..ab8c9d8a 100644
--- a/src/PIL/GifImagePlugin.py
+++ b/src/PIL/GifImagePlugin.py
@@ -619,24 +619,22 @@ def _save_netpbm(im, fp, filename):
     # below for information on how to enable this.
 
     import os
-    from subprocess import Popen, check_call, PIPE, CalledProcessError
+    from subprocess import Popen, check_call, PIPE, CalledProcessError, DEVNULL
 
     tempfile = im._dump()
 
     with open(filename, "wb") as f:
         if im.mode != "RGB":
-            with open(os.devnull, "wb") as devnull:
-                check_call(["ppmtogif", tempfile], stdout=f, stderr=devnull)
+            check_call(["ppmtogif", tempfile], stdout=f, stderr=DEVNULL)
         else:
             # Pipe ppmquant output into ppmtogif
             # "ppmquant 256 %s | ppmtogif > %s" % (tempfile, filename)
             quant_cmd = ["ppmquant", "256", tempfile]
             togif_cmd = ["ppmtogif"]
-            with open(os.devnull, "wb") as devnull:
-                quant_proc = Popen(quant_cmd, stdout=PIPE, stderr=devnull)
-                togif_proc = Popen(
-                    togif_cmd, stdin=quant_proc.stdout, stdout=f, stderr=devnull
-                )
+            quant_proc = Popen(quant_cmd, stdout=PIPE, stderr=DEVNULL)
+            togif_proc = Popen(
+                togif_cmd, stdin=quant_proc.stdout, stdout=f, stderr=DEVNULL
+            )
 
             # Allow ppmquant to receive SIGPIPE if ppmtogif exits
             quant_proc.stdout.close()
diff --git a/src/PIL/IcnsImagePlugin.py b/src/PIL/IcnsImagePlugin.py
index 03484cf0..ddcd98c5 100644
--- a/src/PIL/IcnsImagePlugin.py
+++ b/src/PIL/IcnsImagePlugin.py
@@ -333,11 +333,10 @@ def _save(im, fp, filename):
         last_w = w * 2
 
     # iconutil -c icns -o {} {}
-    from subprocess import Popen, PIPE, CalledProcessError
+    from subprocess import Popen, PIPE, CalledProcessError, DEVNULL
 
     convert_cmd = ["iconutil", "-c", "icns", "-o", filename, iconset]
-    with open(os.devnull, "wb") as devnull:
-        convert_proc = Popen(convert_cmd, stdout=PIPE, stderr=devnull)
+    convert_proc = Popen(convert_cmd, stdout=PIPE, stderr=DEVNULL)
 
     convert_proc.stdout.close()

@hugovk
Copy link
Member Author

hugovk commented Oct 8, 2019

Thanks, simplified in a191976.

@jdufresne
Copy link
Contributor

Very nice. Can now simplify temporary directory cleanup by using tempfile.TemporaryDirectory context manager (since Python 3.2).

diff --git a/Tests/test_file_pdf.py b/Tests/test_file_pdf.py
index 0158807f..3547a593 100644
--- a/Tests/test_file_pdf.py
+++ b/Tests/test_file_pdf.py
@@ -162,13 +162,10 @@ class TestFilePdf(PillowTestCase):
 
     def test_pdf_append_fails_on_nonexistent_file(self):
         im = hopper("RGB")
-        temp_dir = tempfile.mkdtemp()
-        try:
+        with tempfile.TemporaryDirectory() as temp_dir:
             self.assertRaises(
                 IOError, im.save, os.path.join(temp_dir, "nonexistent.pdf"), append=True
             )
-        finally:
-            os.rmdir(temp_dir)
 
     def check_pdf_pages_consistency(self, pdf):
         pages_info = pdf.read_indirect(pdf.pages_ref)
diff --git a/src/PIL/IcnsImagePlugin.py b/src/PIL/IcnsImagePlugin.py
index b80ac217..18e8403d 100644
--- a/src/PIL/IcnsImagePlugin.py
+++ b/src/PIL/IcnsImagePlugin.py
@@ -314,41 +314,40 @@ def _save(im, fp, filename):
         fp.flush()
 
     # create the temporary set of pngs
-    iconset = tempfile.mkdtemp(".iconset")
-    provided_images = {im.width: im for im in im.encoderinfo.get("append_images", [])}
-    last_w = None
-    second_path = None
-    for w in [16, 32, 128, 256, 512]:
-        prefix = "icon_{}x{}".format(w, w)
-
-        first_path = os.path.join(iconset, prefix + ".png")
-        if last_w == w:
-            shutil.copyfile(second_path, first_path)
-        else:
-            im_w = provided_images.get(w, im.resize((w, w), Image.LANCZOS))
-            im_w.save(first_path)
-
-        second_path = os.path.join(iconset, prefix + "@2x.png")
-        im_w2 = provided_images.get(w * 2, im.resize((w * 2, w * 2), Image.LANCZOS))
-        im_w2.save(second_path)
-        last_w = w * 2
-
-    # iconutil -c icns -o {} {}
-
-    convert_cmd = ["iconutil", "-c", "icns", "-o", filename, iconset]
-    convert_proc = subprocess.Popen(
-        convert_cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL
-    )
-
-    convert_proc.stdout.close()
+    with tempfile.TemporaryDirectory(".iconset") as iconset:
+        provided_images = {
+            im.width: im for im in im.encoderinfo.get("append_images", [])
+        }
+        last_w = None
+        second_path = None
+        for w in [16, 32, 128, 256, 512]:
+            prefix = "icon_{}x{}".format(w, w)
+
+            first_path = os.path.join(iconset, prefix + ".png")
+            if last_w == w:
+                shutil.copyfile(second_path, first_path)
+            else:
+                im_w = provided_images.get(w, im.resize((w, w), Image.LANCZOS))
+                im_w.save(first_path)
+
+            second_path = os.path.join(iconset, prefix + "@2x.png")
+            im_w2 = provided_images.get(w * 2, im.resize((w * 2, w * 2), Image.LANCZOS))
+            im_w2.save(second_path)
+            last_w = w * 2
+
+        # iconutil -c icns -o {} {}
+
+        convert_cmd = ["iconutil", "-c", "icns", "-o", filename, iconset]
+        convert_proc = subprocess.Popen(
+            convert_cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL
+        )
 
-    retcode = convert_proc.wait()
+        convert_proc.stdout.close()
 
-    # remove the temporary files
-    shutil.rmtree(iconset)
+        retcode = convert_proc.wait()
 
-    if retcode:
-        raise subprocess.CalledProcessError(retcode, convert_cmd)
+        if retcode:
+            raise subprocess.CalledProcessError(retcode, convert_cmd)
 
 
 Image.register_open(IcnsImageFile.format, IcnsImageFile, lambda x: x[:4] == b"icns")

@jdufresne
Copy link
Contributor

Some uses of __future__ remain in the docs. Might as well conform docs to the same standards and practices.

diff --git a/docs/handbook/tutorial.rst b/docs/handbook/tutorial.rst
index 16090b04..e57bc440 100644
--- a/docs/handbook/tutorial.rst
+++ b/docs/handbook/tutorial.rst
@@ -18,7 +18,6 @@ in the :py:mod:`~PIL.Image` module::
 If successful, this function returns an :py:class:`~PIL.Image.Image` object.
 You can now use instance attributes to examine the file contents::
 
-    >>> from __future__ import print_function
     >>> print(im.format, im.size, im.mode)
     PPM (512, 512) RGB
 
@@ -67,7 +66,6 @@ Convert files to JPEG
 
 ::
 
-    from __future__ import print_function
     import os, sys
     from PIL import Image
 
@@ -89,7 +87,6 @@ Create JPEG thumbnails
 
 ::
 
-    from __future__ import print_function
     import os, sys
     from PIL import Image
 
@@ -120,7 +117,6 @@ Identify Image Files
 
 ::
 
-    from __future__ import print_function
     import sys
     from PIL import Image
 
@@ -513,7 +509,6 @@ This is only available for JPEG and MPO files.
 ::
 
     from PIL import Image
-    from __future__ import print_function
     im = Image.open(file)
     print("original =", im.mode, im.size)

@hugovk
Copy link
Member Author

hugovk commented Nov 1, 2019

Merge conflicts resolved.

@jdufresne
Copy link
Contributor

We can take advantage of Black's --target-version CLI argument now.

diff --git a/src/PIL/ImageDraw.py b/src/PIL/ImageDraw.py
index 3ece2015..c6e12150 100644
--- a/src/PIL/ImageDraw.py
+++ b/src/PIL/ImageDraw.py
@@ -313,7 +313,7 @@ class ImageDraw:
                     language=language,
                     stroke_width=stroke_width,
                     *args,
-                    **kwargs
+                    **kwargs,
                 )
                 coord = coord[0] + offset[0], coord[1] + offset[1]
             except AttributeError:
@@ -326,7 +326,7 @@ class ImageDraw:
                         language,
                         stroke_width,
                         *args,
-                        **kwargs
+                        **kwargs,
                     )
                 except TypeError:
                     mask = font.getmask(text)
diff --git a/src/PIL/ImageFilter.py b/src/PIL/ImageFilter.py
index 9cb62ad2..6b0f5eb3 100644
--- a/src/PIL/ImageFilter.py
+++ b/src/PIL/ImageFilter.py
@@ -495,7 +495,7 @@ class Color3DLUT(MultibandFilter):
                             r / (size1D - 1),
                             g / (size2D - 1),
                             b / (size3D - 1),
-                            *values
+                            *values,
                         )
                     else:
                         values = callback(*values)
diff --git a/tox.ini b/tox.ini
index 0bf833b8..07d75be6 100644
--- a/tox.ini
+++ b/tox.ini
@@ -24,7 +24,7 @@ deps =
 
 [testenv:lint]
 commands =
-    black --check --diff .
+    black --target-version py35 --check --diff .
     flake8 --statistics --count
     isort --check-only --diff
     check-manifest

@hugovk
Copy link
Member Author

hugovk commented Nov 2, 2019

Any objections to merge this?

Would be good to allow master to be tested for longer, and also unblocks #4145, and avoid any further merge conflicts.

@jdufresne
Copy link
Contributor

That makes sense to me. Really nice work here.

I've discovered one more tiny tiny cleanup. In Python 3, the next was renamed to __next__, so can remove the Python 2 shim:

diff --git a/src/PIL/ImageSequence.py b/src/PIL/ImageSequence.py
index 28a54c7b..4e9f5c21 100644
--- a/src/PIL/ImageSequence.py
+++ b/src/PIL/ImageSequence.py
@@ -52,9 +52,6 @@ class Iterator:
         except EOFError:
             raise StopIteration
 
-    def next(self):
-        return self.__next__()
-
 
 def all_frames(im, func=None):
     """

@jdufresne
Copy link
Contributor

jdufresne commented Nov 3, 2019

Because of PEP 393, the internal Unicode representation has been unified. I believe we can now remove the UCS2/UCS4 checks at import. There are more details at sys.maxunicode. Do you agree?

diff --git a/src/PIL/Image.py b/src/PIL/Image.py
index 27777da1..80327717 100644
--- a/src/PIL/Image.py
+++ b/src/PIL/Image.py
@@ -94,22 +94,6 @@ except ImportError as v:
         )
     elif str(v).startswith("The _imaging extension"):
         warnings.warn(str(v), RuntimeWarning)
-    elif "Symbol not found: _PyUnicodeUCS2_" in str(v):
-        # should match _PyUnicodeUCS2_FromString and
-        # _PyUnicodeUCS2_AsLatin1String
-        warnings.warn(
-            "The _imaging extension was built for Python with UCS2 support; "
-            "recompile Pillow or build Python --without-wide-unicode. ",
-            RuntimeWarning,
-        )
-    elif "Symbol not found: _PyUnicodeUCS4_" in str(v):
-        # should match _PyUnicodeUCS4_FromString and
-        # _PyUnicodeUCS4_AsLatin1String
-        warnings.warn(
-            "The _imaging extension was built for Python with UCS4 support; "
-            "recompile Pillow or build Python --with-wide-unicode. ",
-            RuntimeWarning,
-        )
     # Fail here anyway. Don't let people run with a mostly broken Pillow.
     # see docs/porting.rst
     raise

@hugovk
Copy link
Member Author

hugovk commented Nov 3, 2019

Yes, I think that can go too. The Python interpreter pre-3.3 could be compiled with "narrow" or "wide" Unicode. In 3.3 and later, Python decides what sort of encoding to use based on the string, and the user doesn't need to worry about it.

Here's a good explanation behind it and the PEP 393 change in 3.3:

https://www.b-list.org/weblog/2017/sep/05/how-python-does-unicode/

Those error messages first came up when someone managed to compile Python with one "wideness" and Pillow with the other: #297.

@hugovk
Copy link
Member Author

hugovk commented Nov 16, 2019

Merge conflicts resolved.

@hugovk
Copy link
Member Author

hugovk commented Nov 18, 2019

Merge conflicts resolved.

@hugovk
Copy link
Member Author

hugovk commented Nov 20, 2019

Merge conflicts resolved. This PR is blocking #4145.

@hugovk hugovk added the Blocker Blocks another issue (say which one) label Nov 20, 2019
@jdufresne
Copy link
Contributor

jdufresne commented Nov 20, 2019

Unless the project plans to do another Python 2 release before end of year, I think it makes a lot of sense to get this PR landed. It touches many files and lines that require regular merging. Landing now will help avoid more merge conflicts and avoid the repetitive work going into resolving those conflicts. It will also allow new work to be simplified as it need only target Python 3.

@hugovk
Copy link
Member Author

hugovk commented Nov 20, 2019

Agreed. We're not planning any more Python 2 releases, unless some critical bug fix comes up. But if it does it will be from the 6.2.x branch and not master.

@hugovk
Copy link
Member Author

hugovk commented Nov 20, 2019

Time to merge!

Goodbye Python 2, thank you everyone over the years who's helped with Python 2 + 3 compatibility, and especially @fluggo for adding Python 3 support (at that time 3.2 and 3.3) in 2013 (#35), released in Pillow 2.0.0 on 15 March 2013.

PR to add Python 3 PR to remove Python 2
#35 #4109
Commits 61 Commits 26
Files changed 230 Files changed 158
+6,767 −2,096 +594 −1,521

@hugovk hugovk merged commit f90a219 into python-pillow:master Nov 20, 2019
@hugovk hugovk deleted the rm-2.7 branch November 20, 2019 15:23
@hugovk hugovk removed the Blocker Blocks another issue (say which one) label Nov 20, 2019
@fluggo
Copy link
Contributor

fluggo commented Nov 21, 2019

This is beautiful. Congratulations, Pillow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Removal Removal of a feature, usually done in major releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants