Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-74598: addfnmatch.filterfalse for excluding names#121185

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
picnixz merged 21 commits intopython:mainfrompicnixz:fnmatch-filterfalse
Apr 8, 2025

Conversation

@picnixz
Copy link
Member

@picnixzpicnixz commentedJun 30, 2024•
edited
Loading

In this implementation, I did not use a lambda function as it was proposed in the original patch nor did I use auxiliary functions.

EDIT 07/04/2025: I kept the use offilterfalse as on a PGO build the loop variant was much slower when processing past a certain number of files. For < 10 files, the numbers are roughly the same and in the range of nanoseconds so we don't really care.

# 6 files to testsmall_match_none_loop: Mean +- std dev: 628 ns +- 9 nssmall_match_none_iter: Mean +- std dev: 622 ns +- 14 nssmall_match_some_loop: Mean +- std dev: 675 ns +- 12 nssmall_match_some_iter: Mean +- std dev: 688 ns +- 12 nssmall_match_digits_loop: Mean +- std dev: 962 ns +- 15 nssmall_match_digits_iter: Mean +- std dev: 987 ns +- 12 nssmall_match_all_loop: Mean +- std dev: 700 ns +- 7 nssmall_match_all_iter: Mean +- std dev: 765 ns +- 19 ns# 60 files to testmedium_match_none_loop: Mean +- std dev: 4.75 us +- 0.10 usmedium_match_none_iter: Mean +- std dev: 3.85 us +- 0.07 usmedium_match_some_loop: Mean +- std dev: 5.08 us +- 0.07 usmedium_match_some_iter: Mean +- std dev: 4.23 us +- 0.07 usmedium_match_digits_loop: Mean +- std dev: 7.93 us +- 0.15 usmedium_match_digits_iter: Mean +- std dev: 7.12 us +- 0.14 usmedium_match_all_loop: Mean +- std dev: 5.35 us +- 0.12 usmedium_match_all_iter: Mean +- std dev: 4.66 us +- 0.10 us# 600 files to testlarge_match_none_loop: Mean +- std dev: 43.9 us +- 0.9 uslarge_match_none_iter: Mean +- std dev: 33.9 us +- 0.7 uslarge_match_some_loop: Mean +- std dev: 47.4 us +- 1.0 uslarge_match_some_iter: Mean +- std dev: 36.4 us +- 0.7 uslarge_match_digits_loop: Mean +- std dev: 75.4 us +- 1.5 uslarge_match_digits_iter: Mean +- std dev: 64.6 us +- 1.2 uslarge_match_all_loop: Mean +- std dev: 48.2 us +- 0.9 uslarge_match_all_iter: Mean +- std dev: 39.4 us +- 0.7 us
Benchmark script
importitertoolsimportosimportposixpathfromfnmatchimport_compile_patternimportpyperfdeffilterfalse_loop(names,pat):result= []pat=os.path.normcase(pat)match=_compile_pattern(pat)ifos.pathisposixpath:# normcase on posix is NOP. Optimize it away from the loop.fornameinnames:ifmatch(name)isNone:result.append(name)else:fornameinnames:ifmatch(os.path.normcase(name))isNone:result.append(name)returnresultdeffilterfalse_iter(names,pat):pat=os.path.normcase(pat)match=_compile_pattern(pat)ifos.pathisposixpath:# normcase on posix is NOP. Optimize it away from the loop.returnlist(itertools.filterfalse(match,names))result= []fornameinnames:ifmatch(os.path.normcase(name))isNone:result.append(name)returnresultrunner=pyperf.Runner()runner.parse_args()base_data= ['Python','Ruby','Perl','Tcl','123456789_path_with_digits.json','_path_with_digits_123456789.json']fordata_label,data_sizein [('small',1), ('medium',10), ('large',100)]:forpat_label,patternin [('none','A*'), ('some','P*'), ('digits','*[1-9]*'), ('all','*')]:data=base_data*data_sizerunner.bench_func(f'{data_label}_match_{pat_label}_loop',filterfalse_loop,data,pattern)runner.bench_func(f'{data_label}_match_{pat_label}_iter',filterfalse_iter,data,pattern)

📚 Documentation preview 📚:https://cpython-previews--121185.org.readthedocs.build/

@picnixzpicnixz changed the titlegh-74598: addfnmatch.filterfalse for excluding patternsgh-74598: addfnmatch.filterfalse for excluding namesJun 30, 2024
@picnixzpicnixz added the stdlibStandard Library Python modules in the Lib/ directory labelAug 1, 2024
@picnixz
Copy link
MemberAuthor

picnixz commentedMar 1, 2025•
edited
Loading

FT failure is known so I'll wait until the fix (#130724) is merged.

@picnixzpicnixz added the staleStale PR or inactive for long period of time. labelMar 18, 2025
@picnixzpicnixz marked this pull request as draftApril 7, 2025 07:35
@picnixzpicnixz marked this pull request as ready for reviewApril 7, 2025 07:36
@picnixzpicnixz removed the staleStale PR or inactive for long period of time. labelApr 7, 2025
@picnixz
Copy link
MemberAuthor

I'm going to run some benchmarks again and then I'll merge this one. The feature can still be useful and maintenance is not that hard. If, however, benchmarks are not good, I'll write a recipe instead.

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I did have other plans forfnmatch, for supporting complex filters with multiple positive and negative patterns, this is why I was not particularly interested in this feature. But I don't think that addingfilterfalse() is wrong. It will not conflict with future features, and they will not make it obsolete.

LGTM. But please add more tests forfilterfalse() similar to otherfilter() tests inFilterTestCase. The code is completely separate, so we need to test all this.

@picnixz
Copy link
MemberAuthor

But please add more tests for filterfalse() similar to other filter() tests in FilterTestCase

Will do!

@picnixzpicnixzenabled auto-merge (squash)April 8, 2025 09:48
@picnixzpicnixz merged commit3eda146 intopython:mainApr 8, 2025
39 checks passed
@picnixzpicnixz deleted the fnmatch-filterfalse branchApril 8, 2025 10:30
seehwan pushed a commit to seehwan/cpython that referenced this pull requestApr 16, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka left review comments

@AlexWaygoodAlexWaygoodAlexWaygood left review comments

@barneygalebarneygaleAwaiting requested review from barneygale

+1 more reviewer

@dg-pbdg-pbdg-pb left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

stdlibStandard Library Python modules in the Lib/ directory

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@picnixz@dg-pb@serhiy-storchaka@AlexWaygood
[8]ページ先頭
©2009-2025 Movatter.jp