Follow symlinks recursively - #56
kaitj merged 3 commits into
Conversation
Sorry, something went wrong.
The current implementation did not follow symlink directories, only files. The built-in flag 'recurse_symlinks' was not introduced until py3.13. This commit adds a method for safely recursing directories for local paths (assumes we don't have to worry about symlinks for cloudpaths) Wil need to add some proper tests, but interactive testing suggests this fix works. Eventually, this method can be dropped in favor of path.rglob(*, recurse_symlinks=True) when only py3.13+ is supported.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #56 +/- ##
==========================================
+ Coverage 95.07% 95.15% +0.07%
==========================================
Files 7 7
Lines 487 495 +8
==========================================
+ Hits 463 471 +8
Misses 24 24 ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
| @pytest.fixture | ||
| def symlink_dataset(tmp_path: Path, sub: str = "sub-001", ses: str = "ses-001") -> Path: | ||
| data = tmp_path / "data" | ||
| data.mkdir() |
There was a problem hiding this comment.
Does this need to be cleaned up?
Sorry, something went wrong.
There was a problem hiding this comment.
When you say "cleaned up" do you mean the code for the fixture or like the directory itself?
Sorry, something went wrong.
- Use built-in method for ClouthPath's since `glob.glob` does not properly recurse through sub-directories. If it is a local path, just fall back to `glob.glob`. Alternatively, could just check for Python version and only run this if <py3.13. - The built-in rglob methods have slightly different parameters so need different calls for them.
|
@clane9, wondering if you have any thoughts about this before I merge? Might be misremembering, but I think we might have chatted about this briefly at some point. |
Sorry, something went wrong.
|
looks fine to me! |
Sorry, something went wrong.
Resolves #55
The path.glob that was used didn't follow directories that were symlinked. In py3.13, recurse_symlinks argument was introduced, but this is still an issue in py3.11 and py3.12. This introduces a method that performs similarly to py3.13 to recurse symlinks as well via os.walk. From what I was able to figure out, this was only needed when indexing a subject directory. I also added a (I admit messy) pytest fixture for testing indexing with symlinks.
I wonder if there is a better fix / way to do this. Turns out yes...the changes stem from trying to unify pathlib.path.glob with glob.glob (see python/cpython#117589). For now, this is falling back on glob.glob with a recursive pattern to support <py3.13.
@nx10 - mind taking a look? My brain is doing circles trying to sort out this symlinking stuff.