Fix broken external_file paths when organizing on Windows#1832
Fix broken external_file paths when organizing on Windows#1832CodyCBakerPhD merged 3 commits intodandi:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1832 +/- ##
==========================================
- Coverage 76.28% 76.27% -0.02%
==========================================
Files 87 87
Lines 12482 12484 +2
==========================================
Hits 9522 9522
- Misses 2960 2962 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I can't add labels (no write access), a maintainer will need to add patch. The codecov checks are failing but the actual changes shouldn't reduce coverage. |
|
@h-mayorquin Perhaps worth an NWB Inspector check as well? |
|
|
LGTM Thanks for the catch |
|
what's up with the code coverage? |
Something to do with how it counts import lines All source code lines look to be hit |
When
dandi organize --update-external-file-pathsruns on Windows, theexternal_filepaths written into NWB files use backslashes instead of forward slashes.Expected:
On Windows before this fix:
Downstream tools expecting forward slashes (as DANDI/S3 asset keys always use) fail silently when resolving these paths against the archive. This was observed in Dandiset 001771 and reported in catalystneuro/nwb-video-widgets#33, where
get_dandi_video_info()returned empty results. A defensive workaround was merged in catalystneuro/nwb-video-widgets#38, but the root cause is here indandi organize.The fix has two parts:
organize.py):_create_external_file_names()usedos.path.jointo build external file paths. On Windows that produces backslashes. Replaced withposixpath.joinsince these are DANDI/S3 keys, not local filesystem paths.pynwb_utils.py):_get_image_series()wrapped external file strings inPath(), which on Windows converts forward slashes back to backslashes. Replaced withPurePosixPath.I have also added a regression assertion in
test_video_organizethatexternal_filevalues never contain backslashes. The initial push included only the write-side fix; Windows CI caught thatPath()on the read side was re-introducing backslashes, confirming the assertion works as intended.