GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-89727: Fix pathlib.Path.walk RecursionError on deep trees by zmievsa · Pull Request #100282 · python/cpython

Conversation

@zmievsa

@zmievsa zmievsa commented Dec 15, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.

  • Issue: Change shutil.rmtree and os.walk to support very deep hierarchies #89727

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
wimglenn reacted with thumbs up emoji jonburdo and barneygale reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 2 reactions
@zmievsa
Replace recursion with iteration
037a717
@bedevere-bot bedevere-bot mentioned this pull request Dec 15, 2022
Change shutil.rmtree and os.walk to support very deep hierarchies #89727
Open
@netlify

netlify Bot commented Dec 15, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

✅ Deploy Preview for python-cpython-preview canceled.

Name Link
🔨 Latest commit 2bfc47d
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/639c50140be628000884140d
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting review label Dec 15, 2022
@zmievsa
Unify walk and _walk methods
f8968b6
@zmievsa

zmievsa commented Dec 15, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zmievsa and others added 5 commits December 16, 2022 03:10
@zmievsa
Remove empty typehint
ecabf8c
@zmievsa
Fix all tests
6ec3c3e
@zmievsa
Add test for recursion limit
fb5a4c7
@blurb-it
📜🤖 Added by blurb_it.
55beb6e
@zmievsa
Merge branch 'main' into pythongh-89727/fix-pathlib.Path.walk-recursi…
7f36140 
…on-depth
@zmievsa
zmievsa marked this pull request as ready for review December 16, 2022 10:32
@zmievsa
zmievsa requested a review from brettcannon as a code owner December 16, 2022 10:32
zmievsa added 2 commits December 16, 2022 15:01
@zmievsa
Fix recursion limit issue in different envs
508bd4d
@zmievsa
Merge branch 'pythongh-89727/fix-pathlib.Path.walk-recursion-depth' o…
2bfc47d 
…f github.com:ovsyanka83/cpython into pythongh-89727/fix-pathlib.Path.walk-recursion-depth
@pitrou
pitrou requested a review from serhiy-storchaka December 16, 2022 14:27
@brettcannon

brettcannon commented Dec 16, 2022

Copy link
Copy Markdown
Member

/cc @barneygale
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brettcannon brettcannon added the topic-pathlib label Dec 16, 2022
@zmievsa
Make everything consistent with os.walk
20476ff
kalvdans
kalvdans reviewed Dec 20, 2022
View reviewed changes
Comment thread Lib/pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@zmievsa
Remove unused import
4f2de62
carljm
carljm reviewed Dec 20, 2022
View reviewed changes
Comment thread Lib/test/test_pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
zmievsa added 2 commits December 21, 2022 01:47
@zmievsa
Merge branch 'main' into pythongh-89727/fix-pathlib.Path.walk-recursi…
6313b6a 
…on-depth
@zmievsa
Replace the usage of infinite_recursion with set_recursion_limit
17497f0
carljm
carljm approved these changes Dec 20, 2022
View reviewed changes

@carljm carljm left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zmievsa reacted with hooray emoji
All reactions
  • 🎉 1 reaction
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Dec 20, 2022
@zmievsa

zmievsa commented Dec 20, 2022

Copy link
Copy Markdown
Contributor Author

@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.
carljm reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zmievsa

zmievsa commented Dec 24, 2022

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barneygale

barneygale commented Dec 24, 2022

Copy link
Copy Markdown
Contributor

I'll look at this within the next few days!
zmievsa reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
barneygale
barneygale reviewed Jan 2, 2023
View reviewed changes
Comment thread Lib/pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Ovsyanka83 @barneygale
Update Lib/pathlib.py
b7d36de 
Co-authored-by: Barney Gale <[email protected]>
barneygale
barneygale approved these changes Jan 2, 2023
View reviewed changes

@barneygale barneygale left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good to me!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@zmievsa
Code review fixes
d428a89
@zmievsa

zmievsa commented Jan 11, 2023

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zmievsa
Merge branch 'main' into pythongh-89727/fix-pathlib.Path.walk-recursi…
4ce5291 
…on-depth
@AlexWaygood

AlexWaygood commented Jan 11, 2023

Copy link
Copy Markdown
Member

Sorry @Ovsyanka83, I'll try to take a proper look soon!
zmievsa reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
brettcannon
brettcannon approved these changes Jan 21, 2023
View reviewed changes
Comment thread Lib/pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test_pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jan 21, 2023
@Ovsyanka83 @brettcannon
Apply suggestions from code review
49d4237 
Co-authored-by: Brett Cannon <[email protected]>
@barneygale

barneygale commented Feb 1, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

@AlexWaygood gentle nudge to review this when you have the time! Thanks :)
zmievsa reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
CuriousLearner
CuriousLearner approved these changes Feb 18, 2023
View reviewed changes

@CuriousLearner CuriousLearner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@barneygale

barneygale commented Feb 28, 2023

Copy link
Copy Markdown
Contributor

@AlexWaygood https://youtu.be/CdqoNKCCt7A
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
barneygale
barneygale reviewed Mar 11, 2023
View reviewed changes
Comment thread Lib/pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@barneygale barneygale mentioned this pull request Mar 12, 2023
Path.rglob performance issues in deeply nested directories compared to glob.glob(recursive=True) #102613
Closed
@ionite34 ionite34 mentioned this pull request Mar 12, 2023
BOT-3EN: snekbox hits a recusion exceded exception in pathlib when parsing lots of nested directories and 500s python-discord/snekbox#172
Closed
@zmievsa
Rename stack to paths
9bc2500
barneygale
barneygale approved these changes Mar 19, 2023
View reviewed changes
@barneygale
barneygale merged commit 713df2c into python:main Mar 22, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 22, 2023
@zmievsa

zmievsa commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

@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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zmievsa
zmievsa deleted the gh-89727/fix-pathlib.Path.walk-recursion-depth branch March 22, 2023 18:20
@AlexWaygood

AlexWaygood commented Mar 22, 2023

Copy link
Copy Markdown
Member

We made him a core dev! https://discuss.python.org/t/vote-to-promote-barney-gale/24801
zmievsa reacted with hooray emoji zmievsa reacted with heart emoji
All reactions
  • 🎉 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@barneygale

barneygale commented Mar 22, 2023

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!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
@zmievsa @barneygale
@brettcannon @Fidget-Spinner
pythonGH-89727: Fix pathlib.Path.walk RecursionError on deep trees (p…
b645f1b 
…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 <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@zmievsa @barneygale
@brettcannon @warsaw
pythonGH-89727: Fix pathlib.Path.walk RecursionError on deep trees (p…
dfeb972 
…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 <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
barneygale added a commit to barneygale/cpython that referenced this pull request May 11, 2023
@barneygale
pythonGH-102613: Fix recursion error from pathlib.Path.glob()
71dc988 
Use `Path.walk()` to implement the recursive wildcard `**`. This method
uses an iterative (rather than recursive) walk - see pythonGH-100282.
@barneygale barneygale mentioned this pull request May 11, 2023
GH-102613: Fix recursion error from pathlib.Path.glob() #104373
Merged
barneygale added a commit that referenced this pull request May 15, 2023
@barneygale
GH-102613: Fix recursion error from pathlib.Path.glob() (GH-104373)
cb88ae6 
Use `Path.walk()` to implement the recursive wildcard `**`. This method
uses an iterative (rather than recursive) walk - see GH-100282.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannon brettcannon brettcannon approved these changes
@carljm carljm carljm approved these changes
@barneygale barneygale barneygale approved these changes
@CuriousLearner CuriousLearner CuriousLearner approved these changes
@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka
@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood
+1 more reviewer
@kalvdans kalvdans kalvdans left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-pathlib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@zmievsa @brettcannon @barneygale @AlexWaygood @carljm @kalvdans @CuriousLearner @bedevere-bot @Ovsyanka83
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.