Skip to content

gh-160900: support path-like objects in fnmatch filters - #160980

Open
MonikaDvorackova wants to merge 8 commits into
python:mainfrom
MonikaDvorackova:fix/fnmatch-filter-pathlike
Open

MonikaDvorackova wants to merge 8 commits into
python:mainfrom
MonikaDvorackova:fix/fnmatch-filter-pathlike

Conversation

@MonikaDvorackova

@MonikaDvorackova MonikaDvorackova commented Oct 30, 2026 •

Copy link
Copy Markdown

Fixes #160900.

Ensure fnmatch.filter() and fnmatch.filterfalse() normalize every name with os.path.normcase(), including on POSIX. This lets path-like objects such as pathlib.Path work consistently across platforms. Add regression tests for both functions.

@python-cla-bot

python-cla-bot Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app

bedevere-app Bot commented Oct 30, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@picnixz
picnixz self-requested a review November 4, 2026 13:30
Comment thread Lib/test/test_fnmatch.py Resolved
Comment thread Lib/fnmatch.py Resolved
@MonikaDvorackova

Copy link
Copy Markdown
Author

@picnixz

Thank you for the review. I've addressed the requested changes:

  • Moved the path-like tests immediately after their corresponding filter and filterfalse tests.
  • Updated the documentation to describe path-like input support and preservation of the original objects.
  • Benchmarked the existing itertools.filterfalse implementation against a list comprehension and an explicit loop using pyperf, with input sizes of 10, 100, 1,000, and 10,000 elements.

The list comprehension was consistently the fastest, reducing execution time by approximately 18–21% compared with the previous implementation.

I've updated filterfalse() accordingly and removed the unused itertools import.

All 26 test_fnmatch tests pass, and git diff --check reports no issues.

The benchmark results are preliminary measurements from a CPython 3.16 debug build using pyperf --fast, rather than release-build performance figures.

@read-the-docs-community

read-the-docs-community Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

@picnixz

picnixz commented Nov 4, 2026

Copy link
Copy Markdown
Member

The benchmark results are preliminary measurements from a CPython 3.16 debug build using pyperf --fast, rather than release-build performance figures.

Please don't. Use a release build or one with a --enable-optimizations. A debug build serves no purpose in benchmarking in general. And please tell me which samples you used.

Comment thread Doc/library/fnmatch.rst Resolved
Comment thread Doc/library/fnmatch.rst Resolved
Comment thread Doc/library/fnmatch.rst Resolved
Comment thread Doc/library/fnmatch.rst Resolved
Comment thread Lib/test/test_fnmatch.py Resolved
@MonikaDvorackova

MonikaDvorackova commented Nov 4, 2026 •

Copy link
Copy Markdown
Author

Thanks for the review. I've pushed the documentation updates and added test_mix_bytes_str_pathlike to both FilterTestCase and FilterFalseTestCase. All 28 test_fnmatch tests pass.

I also reran the performance comparison using a CPython 3.16 release build rather than a debug build.

The benchmark compares three implementations:

  • itertools.filterfalse with a lambda predicate
  • A list comprehension
  • An explicit for loop

The input consists of filename strings matched against *.py. I tested input sizes of 10, 100, 1,000, and 10,000 elements, with matching proportions of 0%, 50%, and 100%.

Selected results:

Matches Elements filterfalse + lambda List comprehension Explicit loop
50% 10 1.46 µs 1.17 µs 1.19 µs
50% 100 11.8 µs 10.1 µs 10.3 µs
50% 1,000 115 µs 97.9 µs 98.9 µs
50% 10,000 1.14 ms 966 µs 982 µs

The list comprehension was faster than the lambda-based implementation in 11 of the 12 tested scenarios. However, some measurements showed substantial variability, so these results should be treated as preliminary rather than definitive.

I haven't yet reproduced the benchmark scripts from the original filterfalse PR. I'll check those as well to ensure the comparison covers the original payloads.

@picnixz

picnixz commented Nov 4, 2026

Copy link
Copy Markdown
Member

Please don't use an LLM to post your comments. I don't want to communicate with an agent only. YOur comments are not edited at since the markdown contains the HTML itself.

@picnixz

picnixz commented Nov 5, 2026

Copy link
Copy Markdown
Member

Please avoid updating the branches when there is nothing that has changed. It wastes CI resources. Read https://devguide.python.org/getting-started/pull-request-lifecycle/#update-branch-button

@MonikaDvorackova

Copy link
Copy Markdown
Author

Ok. Thanks for the review. I've addressed the feedback, including the documentation changes and the mixed bytes/str/path-like regression tests.

The latest CI run has completed, with all 51 technical checks passing across the supported platforms.

I also wanted to clarify the earlier branch update: it was intended to retry the macOS CI job after the runner lost communication. I'll avoid unnecessary branch updates going forward.

The PR should now be ready for another review.

Comment thread Lib/test/test_fnmatch.py Resolved
Comment thread Lib/test/test_fnmatch.py Resolved
Comment thread Lib/test/test_fnmatch.py Resolved
Comment thread Lib/fnmatch.py Resolved
@MonikaDvorackova

Copy link
Copy Markdown
Author

I pushed the requested changes in b55db66, including the list comprehension in filter(), and resolved the addressed review threads. Tests, lint, and the documentation preview have all passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fnmatch.filter and filterfalse work with pathlikes only on non-Posix platforms

2 participants