to do:

• clean up code
• add some tests

TheBytesWalkTests.walk andBytesFwalkTests.walk wrapper functions actually prevent late modification (after iteration ofdirs has started) because of the encoding/decoding logic they use which setsdirs before iterating over it. These are of course only for testing purposes, but it means that if we want to runtest_walk_late_dirs_modification in them as well as inWalkTests, we'd need an alternatewalk wrapper which this test uses and some logic to handle the bytes results in theBytes... classes - maybe not worth it?

A few more notes for context:

Thefirst commit has more minimal changes similar tothis suggestion. In that commit, because we're using an iterator, the implementation is a bit awkward and inefficient (i.e. only ever callingnext(dirs) once per loop). There's two cases in which we ignore the value given bynext(dirs) and move on:

1. Withtopdown=True, thefollowlinks or not islink(top) check fails
2. The linescandir_it = scandir(top) throws anOSError which is caught (and not re-raised byonerror)
   The subsequent changes handle these more naturally by looping through the iterator instead of callingcontinue to go through the outer loop again.

The subsequent changes also separate the top down and bottom up logic further (even before splitting into separate functions). Note that even before this PR, there was a fair amount of code that only applied when we havetopdown=False:

• Theif isinstance(top, tuple): call at the top of the outer loop
• walk_dirs creation andif isinstance(top, tuple): inside thescandir_it loop
• if topdown: block at the bottom

Do we really want to introduce this much complexity for a completely undocumented and possibly unused feature?

Do we really want to introduce this much complexity for a completely undocumented and possibly unused feature?

Maybe not. I made this PR to see what it would look like, but am not attached to it. If I were designingos.walk from scratch, I wouldn't support this late editing of dirs - at least not in this way where the list can be edited during its iteration. I think this could be reasonably considered behavior that produces an undefined result, or simply considered a bug, although it was never documented as such.

But I strongly suspect it is used out there (maybe even by accident), and consistent behavior is valuable. If we don't support this, it'd be nice to have some sort of warning about the behavior change but I also don't see a reasonable way to do that. (Checking if dirs was modified would be complex or expensive too and kind of weird).

I'd suggest at least noting the change in behavior in release notes if we don't support the old behavior. I also do prefer separating the top down and bottom up logic as in this PR, although that could be a separate matter.

Other considerations:

• Complexity (or at least code change) is a bit less in the first commit here if that seems any better.
• I don't like the idea of copying this mess over topathlib.Path.walk, and I also don't like the idea of having different behavior, especially if this is considered a bug or undefined behavior. It's also useful to keep the code similar between the two for maintenance, understanding logic, future changes, etc

I don't like the idea of copying this mess over to pathlib.Path.walk, and I also don't like the idea of having different behavior, especially if this is considered a bug or undefined behavior. It's also useful to keep the code similar between the two for maintenance, understanding logic, future changes, etc

pathlib's version ofwalk() was mergedonly because it improved the ergonomics ofos.walk(), including handling symlinks to directories more consistently, which isn't possible inos.walk() for backwards compatibility.

Mutatingdirnames after resuming the walk isn't included or even hinted at in the pathlib docs so I don't want to expend any effort supporting it.