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

GH-72904: Simplify implementation of fnmatch.translate() #109879

Closed
wants to merge 2 commits into from

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Sep 26, 2023

Use re to scan shell-style patterns, rather than parsing them by hand in a fat loop. This makes the code twice as slow (!) but arguably more obvious

Use `re.Scanner` to scan shell-style patterns, rather than parsing them
by hand in a fat loop. This makes the code slower (!) but more obvious, and
lays some groundwork for a future `glob.translate()` function.
@barneygale barneygale marked this pull request as ready for review September 26, 2023 02:22
@barneygale barneygale requested a review from jaraco September 26, 2023 02:22
@AA-Turner
Copy link
Member

twice as slow (!)

What are the benchmarks for this? (i.e. are we talking 10μs to 20μs or 250ms to 500ms?) The regression in performance here may not matter if the translation is a sufficiently small part of any reasonable usecase.

@serhiy-storchaka has been working on re performance lately and may have tips?

A

@barneygale
Copy link
Contributor Author

Withdrawing this PR - the performance isn't good enough.

@barneygale barneygale closed this Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants