Skip to content

gh-89727: Fix pathlib.Path.walk RecursionError on deep trees - #100282

Merged
barneygale merged 20 commits into
python:mainfrom
zmievsa:gh-89727/fix-pathlib.Path.walk-recursion-depth
Mar 22, 2023
Merged

gh-89727: Fix pathlib.Path.walk RecursionError on deep trees#100282
barneygale merged 20 commits into
python:mainfrom
zmievsa:gh-89727/fix-pathlib.Path.walk-recursion-depth

Conversation

@zmievsa

@zmievsazmievsa commented Dec 15, 2022

Copy link
Copy Markdown
Contributor

Use a stack to implement pathlib.Path.walk iteratively instead of recursively to avoid hitting recursion limits on deeply nested trees.

@netlify

netlifyBot commented Dec 15, 2022

Copy link
Copy Markdown

Deploy Preview for python-cpython-preview canceled.

NameLink
🔨 Latest commit2bfc47d
🔍 Latest deploy loghttps://app.netlify.com/sites/python-cpython-preview/deploys/639c50140be628000884140d

@zmievsa

zmievsa commented Dec 15, 2022

Copy link
Copy Markdown
ContributorAuthor

TODO:

  • Add a news blip
  • Add a test that checks for deeply nested directories
  • Figure out a better name for is_result

Update:
All done!

@zmievsa
zmievsa marked this pull request as ready for review December 16, 2022 10:32
@brettcannon

Copy link
Copy Markdown
Member

/cc @barneygale

Comment threadLib/pathlib.py Outdated
Comment threadLib/test/test_pathlib.py Outdated

@carljmcarljm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I assume because of the initial underscore we don't have to worry about people who may have subclassed Path and overridden the _walk method (now to no effect.)

@zmievsa

Copy link
Copy Markdown
ContributorAuthor

@carljm remember that Path.walk has only been created in 3.12 which means that 99% of libraries/projects do not and cannot depend on it yet.

@zmievsa

Copy link
Copy Markdown
ContributorAuthor

@barneygale any estimates of when you will have time to take a look at it? No pressure or rush though.

@barneygale

Copy link
Copy Markdown
Contributor

I'll look at this within the next few days!

Comment threadLib/pathlib.py Outdated
Co-authored-by: Barney Gale <barney.gale@gmail.com>

@barneygalebarneygale left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

@zmievsa

Copy link
Copy Markdown
ContributorAuthor

@AlexWaygood what would be the next steps for this PR? I am ready to continue improving it

@AlexWaygood

Copy link
Copy Markdown
Member

Sorry @Ovsyanka83, I'll try to take a proper look soon!

Comment threadLib/pathlib.py Outdated
Comment threadLib/test/test_pathlib.py Outdated
Co-authored-by: Brett Cannon <brett@python.org>
@barneygale

barneygale commented Feb 1, 2023

Copy link
Copy Markdown
Contributor

@AlexWaygood gentle nudge to review this when you have the time! Thanks :)

@CuriousLearnerCuriousLearner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

Thanks for your contribution.

@barneygale

Copy link
Copy Markdown
Contributor

Comment threadLib/pathlib.py Outdated
@barneygale
barneygale merged commit 713df2c into python:mainMar 22, 2023
@zmievsa

Copy link
Copy Markdown
ContributorAuthor

@barneygale thanks!

Wow. Have you received merge rights? Congrats! Did they make you a core developer or is that a special rule just for you and pathlib?

@zmievsa
zmievsa deleted the gh-89727/fix-pathlib.Path.walk-recursion-depth branch March 22, 2023 18:20
@AlexWaygood

Copy link
Copy Markdown
Member

We made him a core dev! https://discuss.python.org/t/vote-to-promote-barney-gale/24801

@barneygale

Copy link
Copy Markdown
Contributor

@Ovsyanka83 thanks for your patience and for working on this, it's a neat patch :). I'm hoping to build an iterative version of glob() atop!

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
…ythonGH-100282)
Use a stack to implement `pathlib.Path.walk()` iteratively instead of recursively to avoid hitting recursion limits on deeply nested trees.
Co-authored-by: Barney Gale <barney.gale@gmail.com>
Co-authored-by: Brett Cannon <brett@python.org>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
…ythonGH-100282)
Use a stack to implement `pathlib.Path.walk()` iteratively instead of recursively to avoid hitting recursion limits on deeply nested trees.
Co-authored-by: Barney Gale <barney.gale@gmail.com>
Co-authored-by: Brett Cannon <brett@python.org>
barneygale added a commit to barneygale/cpython that referenced this pull request May 11, 2023
Use `Path.walk()` to implement the recursive wildcard `**`. This method
uses an iterative (rather than recursive) walk - see pythonGH-100282.
barneygale added a commit that referenced this pull request May 15, 2023
Use `Path.walk()` to implement the recursive wildcard `**`. This method
uses an iterative (rather than recursive) walk - see GH-100282.
Sign up for freeto 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.

9 participants

@zmievsa@brettcannon@barneygale@AlexWaygood@carljm@kalvdans@CuriousLearner@bedevere-bot@Ovsyanka83