FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-89727: Fix pathlib.Path.walk RecursionError on deep trees by zmievsa · Pull Request #100282 · python/cpython · GitHub

/ cpython Public

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
ConversationCommits (20)ChecksFiles changed
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 commented Dec 15, 2022 •
edited
Loading

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.

bedevere-bot mentioned this pull request Dec 15, 2022
Open

netlify Bot commented Dec 15, 2022 •
edited
Loading

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

zmievsa commented Dec 15, 2022 •
edited
Loading

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!

zmievsa marked this pull request as ready for review December 16, 2022 10:32
zmievsa requested a review from brettcannon as a code owner December 16, 2022 10:32

brettcannon commented Dec 16, 2022

Copy link
Copy Markdown
Member

/cc @barneygale

kalvdans reviewed Dec 20, 2022
View reviewed changes
Comment thread Lib/pathlib.py Outdated
carljm reviewed Dec 20, 2022
View reviewed changes
Comment thread Lib/test/test_pathlib.py Outdated
carljm approved these changes Dec 20, 2022
View reviewed changes

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

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.

barneygale commented Dec 24, 2022

Copy link
Copy Markdown
Contributor

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

barneygale reviewed Jan 2, 2023
View reviewed changes
Comment thread Lib/pathlib.py Outdated
Update Lib/pathlib.py
b7d36de 
Co-authored-by: Barney Gale <barney.gale@gmail.com>
barneygale approved these changes Jan 2, 2023
View reviewed changes

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good to me!

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

AlexWaygood commented Jan 11, 2023

Copy link
Copy Markdown
Member

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

brettcannon approved these changes Jan 21, 2023
View reviewed changes
Comment thread Lib/pathlib.py Outdated
Comment thread Lib/test/test_pathlib.py Outdated
Apply suggestions from code review
49d4237 
Co-authored-by: Brett Cannon <brett@python.org>

barneygale commented Feb 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

CuriousLearner approved these changes Feb 18, 2023
View reviewed changes

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

Thanks for your contribution.

barneygale commented Feb 28, 2023

Copy link
Copy Markdown
Contributor

@AlexWaygood https://youtu.be/CdqoNKCCt7A

barneygale reviewed Mar 11, 2023
View reviewed changes
Comment thread Lib/pathlib.py Outdated
barneygale mentioned this pull request Mar 12, 2023
Closed
ionite34 mentioned this pull request Mar 12, 2023
Closed
barneygale merged commit 713df2c into python:main Mar 22, 2023

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?

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

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

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!

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this pull request Mar 27, 2023
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 <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
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 <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
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 mentioned this pull request May 11, 2023
Merged
barneygale added a commit that referenced this pull request May 15, 2023
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Assignees

No one assigned

Labels

topic-pathlib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL