From 9de5685d7ccc054ed632acebaef57e352e1a4a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 18 Aug 2024 12:34:25 +0200 Subject: [PATCH 01/11] add tests for platform dependent `fnmatch.filter` behaviours --- Lib/fnmatch.py | 4 +++- Lib/test/test_fnmatch.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Lib/fnmatch.py b/Lib/fnmatch.py index 73acb1fe8d4106..fbd2d61422b72c 100644 --- a/Lib/fnmatch.py +++ b/Lib/fnmatch.py @@ -51,7 +51,9 @@ def filter(names, pat): pat = os.path.normcase(pat) match = _compile_pattern(pat) if os.path is posixpath: - # normcase on posix is NOP. Optimize it away from the loop. + # While normcase on POSIX is os.fspath, each NAME is assumed + # to have the same type as PAT, namely a str or bytes object. + # In particular, os.path.normcase is optimized away from the loop. for name in names: if match(name): result.append(name) diff --git a/Lib/test/test_fnmatch.py b/Lib/test/test_fnmatch.py index 10ed496d4e2f37..d1e377e9dd015c 100644 --- a/Lib/test/test_fnmatch.py +++ b/Lib/test/test_fnmatch.py @@ -5,7 +5,9 @@ import string import warnings +from pathlib import Path from fnmatch import fnmatch, fnmatchcase, translate, filter +from test import support class FnmatchTestCase(unittest.TestCase): @@ -262,6 +264,20 @@ def test_mix_bytes_str(self): self.assertRaises(TypeError, filter, ['test'], b'*') self.assertRaises(TypeError, filter, [b'test'], '*') + def test_path_like_objects(self): + path = Path(__file__) + + if support.MS_WINDOWS: + # On non-POSIX platforms, we call os.path.normcase, which + # itself calls os.fspath, thus allowing path-like objects. + self.assertListEqual(filter([path], '*'), [path]) + self.assertListEqual(filter([path], b'*'), [path]) + else: + # On POSIX platforms, we assume that os.path.normcase is + # a no-op, thereby rejecting path-like objects. + self.assertRaises(TypeError, filter, [path], '*') + self.assertRaises(TypeError, filter, [path], b'*') + def test_case(self): ignorecase = os.path.normcase('P') == os.path.normcase('p') self.assertEqual(filter(['Test.py', 'Test.rb', 'Test.PL'], '*.p*'), From 2d1523cdc95b940d9610acda415603a2c7c02751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 18 Aug 2024 12:38:06 +0200 Subject: [PATCH 02/11] update comment --- Lib/fnmatch.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/fnmatch.py b/Lib/fnmatch.py index fbd2d61422b72c..f24a5200b5f669 100644 --- a/Lib/fnmatch.py +++ b/Lib/fnmatch.py @@ -51,9 +51,9 @@ def filter(names, pat): pat = os.path.normcase(pat) match = _compile_pattern(pat) if os.path is posixpath: - # While normcase on POSIX is os.fspath, each NAME is assumed - # to have the same type as PAT, namely a str or bytes object. - # In particular, os.path.normcase is optimized away from the loop. + # While os.path.normcase on POSIX is os.fspath, each name in NAMES is + # assumed to be of the same type as PAT, namely a str or bytes object. + # In particular, os.path.normcase can be optimized away from the loop. for name in names: if match(name): result.append(name) From f1ea74cd3e47d878fc7c03f3a5fde66f4588486a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 18 Aug 2024 13:50:39 +0200 Subject: [PATCH 03/11] Update Lib/test/test_fnmatch.py --- Lib/test/test_fnmatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_fnmatch.py b/Lib/test/test_fnmatch.py index d1e377e9dd015c..32898e2aa46213 100644 --- a/Lib/test/test_fnmatch.py +++ b/Lib/test/test_fnmatch.py @@ -271,7 +271,7 @@ def test_path_like_objects(self): # On non-POSIX platforms, we call os.path.normcase, which # itself calls os.fspath, thus allowing path-like objects. self.assertListEqual(filter([path], '*'), [path]) - self.assertListEqual(filter([path], b'*'), [path]) + self.assertRaises(TypeError, filter, [path], b'*') else: # On POSIX platforms, we assume that os.path.normcase is # a no-op, thereby rejecting path-like objects. From 1fc31873a77699fd3b5a4e318ba1605a27f1432c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 18 Aug 2024 13:52:22 +0200 Subject: [PATCH 04/11] Update Lib/test/test_fnmatch.py --- Lib/test/test_fnmatch.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_fnmatch.py b/Lib/test/test_fnmatch.py index 32898e2aa46213..6ed424fceb5957 100644 --- a/Lib/test/test_fnmatch.py +++ b/Lib/test/test_fnmatch.py @@ -271,12 +271,11 @@ def test_path_like_objects(self): # On non-POSIX platforms, we call os.path.normcase, which # itself calls os.fspath, thus allowing path-like objects. self.assertListEqual(filter([path], '*'), [path]) - self.assertRaises(TypeError, filter, [path], b'*') else: # On POSIX platforms, we assume that os.path.normcase is # a no-op, thereby rejecting path-like objects. self.assertRaises(TypeError, filter, [path], '*') - self.assertRaises(TypeError, filter, [path], b'*') + self.assertRaises(TypeError, filter, [path], b'*') def test_case(self): ignorecase = os.path.normcase('P') == os.path.normcase('p') From efc56f4957bae8a894bc84e35160d46cccc93e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 19 Aug 2024 09:34:46 +0200 Subject: [PATCH 05/11] update implementation --- Lib/fnmatch.py | 15 +++------------ Lib/test/test_fnmatch.py | 11 ++--------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/Lib/fnmatch.py b/Lib/fnmatch.py index f24a5200b5f669..a9a4b635a47ebe 100644 --- a/Lib/fnmatch.py +++ b/Lib/fnmatch.py @@ -10,7 +10,6 @@ corresponding to PATTERN. (It does not compile it.) """ import os -import posixpath import re import functools @@ -50,17 +49,9 @@ def filter(names, pat): result = [] pat = os.path.normcase(pat) match = _compile_pattern(pat) - if os.path is posixpath: - # While os.path.normcase on POSIX is os.fspath, each name in NAMES is - # assumed to be of the same type as PAT, namely a str or bytes object. - # In particular, os.path.normcase can be optimized away from the loop. - for name in names: - if match(name): - result.append(name) - else: - for name in names: - if match(os.path.normcase(name)): - result.append(name) + for name in names: + if match(os.path.normcase(name)): + result.append(name) return result def fnmatchcase(name, pat): diff --git a/Lib/test/test_fnmatch.py b/Lib/test/test_fnmatch.py index 6ed424fceb5957..6008eade33af8e 100644 --- a/Lib/test/test_fnmatch.py +++ b/Lib/test/test_fnmatch.py @@ -266,15 +266,8 @@ def test_mix_bytes_str(self): def test_path_like_objects(self): path = Path(__file__) - - if support.MS_WINDOWS: - # On non-POSIX platforms, we call os.path.normcase, which - # itself calls os.fspath, thus allowing path-like objects. - self.assertListEqual(filter([path], '*'), [path]) - else: - # On POSIX platforms, we assume that os.path.normcase is - # a no-op, thereby rejecting path-like objects. - self.assertRaises(TypeError, filter, [path], '*') + self.assertListEqual(filter([path], '*'), [path]) + # os.path.normcase() always returns a string self.assertRaises(TypeError, filter, [path], b'*') def test_case(self): From 7407728d8e5220c741c7fee889a32adf75532176 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 19 Aug 2024 09:36:54 +0200 Subject: [PATCH 06/11] blurb --- .../next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst diff --git a/Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst b/Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst new file mode 100644 index 00000000000000..6a13ad7ba3b3b7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst @@ -0,0 +1,2 @@ +Fix :term:`path-like objects ` support in +:func:`fnmatch.filter`. Patch by Bénédikt Tran. From bb9a681bafb68cd8007f096af0ae159b6dbd9387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 19 Aug 2024 11:13:56 +0200 Subject: [PATCH 07/11] update comment --- Lib/test/test_fnmatch.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_fnmatch.py b/Lib/test/test_fnmatch.py index 6008eade33af8e..03d976e389bcd2 100644 --- a/Lib/test/test_fnmatch.py +++ b/Lib/test/test_fnmatch.py @@ -267,7 +267,8 @@ def test_mix_bytes_str(self): def test_path_like_objects(self): path = Path(__file__) self.assertListEqual(filter([path], '*'), [path]) - # os.path.normcase() always returns a string + # os.path.normcase() always returns a string for Path objects + # since Path objects expect strings paths and not bytes ones. self.assertRaises(TypeError, filter, [path], b'*') def test_case(self): From cecd642fbad77623677ff011a11fd3b2069eee14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 21 Aug 2024 09:27:45 +0200 Subject: [PATCH 08/11] micro-optimization --- Lib/fnmatch.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Lib/fnmatch.py b/Lib/fnmatch.py index a9a4b635a47ebe..6b798b906e1c95 100644 --- a/Lib/fnmatch.py +++ b/Lib/fnmatch.py @@ -46,13 +46,10 @@ def _compile_pattern(pat): def filter(names, pat): """Construct a list from those elements of the iterable NAMES that match PAT.""" - result = [] - pat = os.path.normcase(pat) + normcase = os.path.normcase + pat = normcase(pat) match = _compile_pattern(pat) - for name in names: - if match(os.path.normcase(name)): - result.append(name) - return result + return [name for name in names if match(normcase(name))] def fnmatchcase(name, pat): """Test whether FILENAME matches PATTERN, including case. From fbca8eaabc63f911ca3a41a571e34f12adbdf262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 21 Aug 2024 09:30:08 +0200 Subject: [PATCH 09/11] blurb --- .../Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst b/Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst index 6a13ad7ba3b3b7..1bf53c1bc4e9fe 100644 --- a/Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst +++ b/Misc/NEWS.d/next/Library/2024-08-19-09-36-50.gh-issue-123135.w_ZNAs.rst @@ -1,2 +1,3 @@ -Fix :term:`path-like objects ` support in -:func:`fnmatch.filter`. Patch by Bénédikt Tran. +Added support for supplying :term:`path-like objects ` +to the *names* parameter of :func:`fnmatch.filter`. Previously, such +objects were only accepted on non-POSIX platforms. Patch by Bénédikt Tran. From 6c92d5a944642b690159fea20f3344b0370a9c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 21 Aug 2024 09:32:40 +0200 Subject: [PATCH 10/11] update docs --- Doc/library/fnmatch.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Doc/library/fnmatch.rst b/Doc/library/fnmatch.rst index fda44923f204fc..f3458cb5b8b7f1 100644 --- a/Doc/library/fnmatch.rst +++ b/Doc/library/fnmatch.rst @@ -83,6 +83,10 @@ cache the compiled regex patterns in the following functions: :func:`fnmatch`, It is the same as ``[n for n in names if fnmatch(n, pat)]``, but implemented more efficiently. + .. versionchanged:: 3.14 + Added support for :term:`path-like objects ` for + the *names* parameter. + .. function:: translate(pat) From cc6548ec9aba50d2c3b7df4de4212522a3d8156a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 22 Aug 2024 09:56:25 +0200 Subject: [PATCH 11/11] improve test coverage --- Lib/test/support/os_helper.py | 3 +++ Lib/test/test_fnmatch.py | 31 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/Lib/test/support/os_helper.py b/Lib/test/support/os_helper.py index 891405943b78c5..726d2ad1ef461b 100644 --- a/Lib/test/support/os_helper.py +++ b/Lib/test/support/os_helper.py @@ -600,6 +600,9 @@ def __init__(self, path): def __repr__(self): return f'' + def __eq__(self, other): + return isinstance(other, type(self)) and self.path == other.path + def __fspath__(self): if (isinstance(self.path, BaseException) or isinstance(self.path, type) and diff --git a/Lib/test/test_fnmatch.py b/Lib/test/test_fnmatch.py index 03d976e389bcd2..7ede2d6b622cc1 100644 --- a/Lib/test/test_fnmatch.py +++ b/Lib/test/test_fnmatch.py @@ -4,10 +4,10 @@ import os import string import warnings - from pathlib import Path +from test.support.os_helper import FakePath + from fnmatch import fnmatch, fnmatchcase, translate, filter -from test import support class FnmatchTestCase(unittest.TestCase): @@ -255,21 +255,26 @@ def test_translate(self): class FilterTestCase(unittest.TestCase): def test_filter(self): - self.assertEqual(filter(['Python', 'Ruby', 'Perl', 'Tcl'], 'P*'), - ['Python', 'Perl']) - self.assertEqual(filter([b'Python', b'Ruby', b'Perl', b'Tcl'], b'P*'), - [b'Python', b'Perl']) + for cls in (str, Path, FakePath): + names = list(map(cls, ['Python', 'Ruby', 'Perl', 'Tcl'])) + filtered = list(map(cls, ['Python', 'Perl'])) + with self.subTest('string pattern', cls=cls, names=names): + self.assertListEqual(filter(names, 'P*'), filtered) + # We want to test the case when os.path.normcase() returns bytes but + # we cannot use pathlib.Path since they only accept string objects. + for cls in (bytes, FakePath): + names = list(map(cls, [b'Python', b'Ruby', b'Perl', b'Tcl'])) + filtered = list(map(cls, [b'Python', b'Perl'])) + with self.subTest('bytes pattern', cls=cls, names=names): + self.assertListEqual(filter(names, b'P*'), filtered) def test_mix_bytes_str(self): self.assertRaises(TypeError, filter, ['test'], b'*') self.assertRaises(TypeError, filter, [b'test'], '*') - - def test_path_like_objects(self): - path = Path(__file__) - self.assertListEqual(filter([path], '*'), [path]) - # os.path.normcase() always returns a string for Path objects - # since Path objects expect strings paths and not bytes ones. - self.assertRaises(TypeError, filter, [path], b'*') + # We want to test the case when os.path.normcase() returns bytes but + # we cannot use pathlib.Path since they only accept string objects. + self.assertRaises(TypeError, filter, [FakePath('test')], b'*') + self.assertRaises(TypeError, filter, [FakePath(b'test')], '*') def test_case(self): ignorecase = os.path.normcase('P') == os.path.normcase('p')