Skip to content

Keep directories holding deeper files in a recursive MemoryFileSystem.rm with maxdepth - #2244

Merged
martindurant merged 1 commit into
fsspec:masterfrom
raashish1601:fix/memory-rm-maxdepth
Oct 8, 2026
Merged

martindurant merged 1 commit into
fsspec:masterfrom
raashish1601:fix/memory-rm-maxdepth

Conversation

@raashish1601

Copy link
Copy Markdown
Contributor

MemoryFileSystem.rm(path, recursive=True, maxdepth=...) raises when a directory at the last expanded level still holds files below maxdepth:

m = fsspec.filesystem("memory")
m.mkdir("/src/sub/deeper")
m.pipe({"/src/top.txt": b"", "/src/sub/mid.txt": b"", "/src/sub/deeper/low.txt": b""})
m.rm("/src", recursive=True, maxdepth=2)
# OSError: Directory not empty: '/src/sub/deeper'

Without the mkdir (implicit directories) it raises FileNotFoundError: /src/sub/deeper instead.

expand_path stops at maxdepth, so /src/sub/deeper is in the list but low.txt is not, and rmdir refuses the non-empty directory. Before raising it has already deleted top.txt and mid.txt.

#2238 made LocalFileSystem.rm keep a directory that still holds deeper files in this case. This does the same for the memory filesystem: when maxdepth is given, a directory that is not empty after the files above it are removed is kept. Without maxdepth, or for a non-recursive rm of a non-empty directory, nothing changes.

Tests: added test_rm_recursive_maxdepth_keeps_deeper_files to test_memory.py (with implicit and explicit directories), mirroring the local test. Both cases fail on master and pass here. test_memory.py and the memory abstract tests pass, and ruff/codespell are clean. Added a changelog entry.

….rm with maxdepth

expand_path stops at maxdepth, so a directory at the last level can still
hold files below it, and rmdir raised OSError for it. Skip such
directories, as LocalFileSystem.rm does.
@raashish1601

Copy link
Copy Markdown
Contributor Author

The pytest-win failures are the three test_github.py tests hitting the GitHub API rate limit (403 rate limit exceeded); they don't touch the memory filesystem. A re-run should clear them.

@martindurant
martindurant merged commit 5d48665 into fsspec:master Oct 8, 2026
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants