Skip to content

Add os.walk wrapper #193

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Add os.walk wrapper #193

wants to merge 4 commits into from

Conversation

stankudrow
Copy link
Contributor

@stankudrow stankudrow commented Nov 11, 2024 •
edited
Loading

Closes #160 .
Also may close #167 .

Features:

  • add an async version of os.walk with tests
  • specify the --maxfail=1 option for pytest
  • update .gitignore
  • update Makefile

@stankudrow
Copy link
Contributor Author

stankudrow commented Nov 12, 2024 •
edited
Loading

Blocked by the PR #192 .

Update: not any more.

@stankudrow stankudrow marked this pull request as draft November 12, 2024 08:28
@stankudrow stankudrow marked this pull request as ready for review November 12, 2024 09:14
@stankudrow stankudrow changed the title Add os.listdir and os.walk wrappers Add os.walk wrapper Nov 12, 2024
@stankudrow stankudrow mentioned this pull request Nov 12, 2024
Open
@stankudrow
Copy link
Contributor Author

#160 (comment) - the os.listdir already wrapped, the os.walk added.

@stankudrow
Copy link
Contributor Author

stankudrow commented Nov 12, 2024 •
edited
Loading

@Tinche , hello. Rebased and ready for review, all checks have been passed.

Update: I found the issue #167 on an "invented-here" solution, what are your suggestions?

@Tinche
Copy link
Owner

Tinche commented Dec 4, 2024

Apologies. I have a small baby nowadays.

So the original sync os.walk is a generator, right? This has some very useful properties which we should keep. Can we make the async version an async generator? Each step can do a step of the sync one.

@stankudrow
Copy link
Contributor Author

Apologies. I have a small baby nowadays.

So the original sync os.walk is a generator, right? This has some very useful properties which we should keep. Can we make the async version an async generator? Each step can do a step of the sync one.

Hello and no need for apologies, au contraire, congratulations.

stankudrow
stankudrow commented Dec 8, 2024
View reviewed changes
Tinche
Tinche requested changes Dec 27, 2024
View reviewed changes
@stankudrow stankudrow requested a review from Tinche December 28, 2024 19:15
@stankudrow stankudrow mentioned this pull request Feb 1, 2025
Merged
@stankudrow stankudrow marked this pull request as draft February 1, 2025 17:22
@stankudrow
Copy link
Contributor Author

Waits for the #199 to be merged.

This was referenced Feb 1, 2025
Closed
Merged
@stankudrow stankudrow marked this pull request as ready for review February 8, 2025 15:55
@stankudrow stankudrow marked this pull request as draft February 8, 2025 16:27
@stankudrow stankudrow marked this pull request as ready for review February 8, 2025 17:44
@stankudrow stankudrow mentioned this pull request Feb 10, 2025
Merged
@Martin005
Copy link

Waits for the #199 to be merged.

That PR has been merged now, can we please merge this one as well? 🙂

@stankudrow stankudrow marked this pull request as draft April 14, 2025 07:41
@stankudrow
Implement 'aiofiles.os.walk' via a thread-safe queue and generators (#1)
b6482e0 
* implement 'aiofiles.os.walk' via Queue and generators

* ignore some mypy and ruff stuff

* implement the async 'os.walk' coro with a thread-safe Queue

* add aiofiles.os.walk tests

* add the 'format' rule

* improve the test_os.py::test_walk_non_blocking case

* improve the test_os.py::test_walk_non_blocking case

---------

Co-authored-by: stankudrow <[email protected]>
Co-authored-by: Tinche <[email protected]>
@stankudrow stankudrow marked this pull request as ready for review April 25, 2025 06:03
@stankudrow
Copy link
Contributor Author

stankudrow commented Apr 25, 2025 •
edited
Loading

Waits for the #199 to be merged.

That PR has been merged now, can we please merge this one as well? 🙂

Good to go after the #213 .

@stankudrow stankudrow mentioned this pull request Apr 26, 2025
Open
@stankudrow stankudrow marked this pull request as draft April 30, 2025 02:36
@stankudrow stankudrow mentioned this pull request May 30, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tinche Tinche Awaiting requested review from Tinche

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Async os.walk attempted solution add "aiofiles.path.walk" or "aiofiles.path.listdir"
3 participants
@stankudrow @Tinche @Martin005