From adae7ecc6a3c61c0850dbf62ae5807bba3c9df32 Mon Sep 17 00:00:00 2001 From: Will Badart Date: Mon, 28 Jan 2019 12:14:42 -0500 Subject: [PATCH 1/4] _util.isPath returns True for pathlib.Path objects Now, for functions which accept either a path or file object, the predicate will pass on Paths and not attempt to call .read on them before opening. The pathlib module was added in 3.4 but os.path functions did not start accepting path-like objects until 3.6, so that is the version after which this implementation is defined. Added a unit test to make sure isPath accepts Path objects. The unit test is skipped if python version is not 3.6 or later. --- Tests/test_util.py | 16 ++++++++++++++++ src/PIL/_util.py | 11 +++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Tests/test_util.py b/Tests/test_util.py index 2316d3d65..dc18ba1e7 100644 --- a/Tests/test_util.py +++ b/Tests/test_util.py @@ -1,7 +1,11 @@ +import sys + from helper import unittest, PillowTestCase from PIL import _util +py36 = sys.version_info.major >= 3 and sys.version_info.minor >= 6 + class TestUtil(PillowTestCase): @@ -35,6 +39,18 @@ class TestUtil(PillowTestCase): # Assert self.assertTrue(it_is) + @unittest.skipIf(not py36, 'os.path support for Paths added in 3.6') + def test_path_obj_is_path(self): + # Arrange + from pathlib import Path + fp = Path('filename.ext') + + # Act + it_is = _util.isPath(fp) + + # Assert + self.assertTrue(it_is) + def test_is_not_path(self): # Arrange filename = self.tempfile("temp.ext") diff --git a/src/PIL/_util.py b/src/PIL/_util.py index 5828c2c24..d2062cb2f 100644 --- a/src/PIL/_util.py +++ b/src/PIL/_util.py @@ -2,13 +2,20 @@ import os import sys py3 = sys.version_info.major >= 3 +py36 = py3 and sys.version_info.minor >= 6 if py3: def isStringType(t): return isinstance(t, str) - def isPath(f): - return isinstance(f, (bytes, str)) + if py36: + from pathlib import Path + + def isPath(f): + return isinstance(f, (bytes, str, Path)) + else: + def isPath(f): + return isinstance(f, (bytes, str)) else: def isStringType(t): return isinstance(t, basestring) # noqa: F821 From 07bff3e9b8a34c85c8326525bd56b02d819a3352 Mon Sep 17 00:00:00 2001 From: Will Badart Date: Mon, 28 Jan 2019 19:45:53 -0500 Subject: [PATCH 2/4] Implement @hugovk's comments The `py36` flag now uses a tuple comparison to correctly handle future major version. The unit test file also now uses `py36` as exported by the _util module, rather than re-testing `sys.version_info`. --- Tests/test_util.py | 4 +--- src/PIL/_util.py | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Tests/test_util.py b/Tests/test_util.py index dc18ba1e7..6d7bfeeb0 100644 --- a/Tests/test_util.py +++ b/Tests/test_util.py @@ -4,8 +4,6 @@ from helper import unittest, PillowTestCase from PIL import _util -py36 = sys.version_info.major >= 3 and sys.version_info.minor >= 6 - class TestUtil(PillowTestCase): @@ -39,7 +37,7 @@ class TestUtil(PillowTestCase): # Assert self.assertTrue(it_is) - @unittest.skipIf(not py36, 'os.path support for Paths added in 3.6') + @unittest.skipIf(not _util.py36, 'os.path support for Paths added in 3.6') def test_path_obj_is_path(self): # Arrange from pathlib import Path diff --git a/src/PIL/_util.py b/src/PIL/_util.py index d2062cb2f..cb307050c 100644 --- a/src/PIL/_util.py +++ b/src/PIL/_util.py @@ -2,7 +2,7 @@ import os import sys py3 = sys.version_info.major >= 3 -py36 = py3 and sys.version_info.minor >= 6 +py36 = sys.version_info[0:2] >= (3, 6) if py3: def isStringType(t): From c328ecace3b28bc25c287401b4415ad2ad718b69 Mon Sep 17 00:00:00 2001 From: Will Badart Date: Mon, 28 Jan 2019 20:08:25 -0500 Subject: [PATCH 3/4] Fix lint error Removed missing import in test_util.py. Stopped needing it after I started reusing the py36 test from the _util module. --- Tests/test_util.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tests/test_util.py b/Tests/test_util.py index 6d7bfeeb0..ea4a2f962 100644 --- a/Tests/test_util.py +++ b/Tests/test_util.py @@ -1,5 +1,3 @@ -import sys - from helper import unittest, PillowTestCase from PIL import _util From aed56efa5050d032af3afe293142483cb53e125a Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 4 Feb 2019 10:36:34 -0500 Subject: [PATCH 4/4] Apply suggestions from code review Rename `fp` to `test_path` in the new `test_is_path` test. ^ Wow, what a sentence... Co-Authored-By: wbadart --- Tests/test_util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/test_util.py b/Tests/test_util.py index ea4a2f962..86999c89f 100644 --- a/Tests/test_util.py +++ b/Tests/test_util.py @@ -39,10 +39,10 @@ class TestUtil(PillowTestCase): def test_path_obj_is_path(self): # Arrange from pathlib import Path - fp = Path('filename.ext') + test_path = Path('filename.ext') # Act - it_is = _util.isPath(fp) + it_is = _util.isPath(test_path) # Assert self.assertTrue(it_is)