Preserve source extension in symlink writer (OME-Zarr support) - #19
Preserve source extension in symlink writer (OME-Zarr support)#19Arshya-Guru wants to merge 2 commits into
Conversation
The symlink backend hardcoded .ims as the link extension, so OME-Zarr sources (which the README already documents as supported) produced links named _SPIM.ims pointing at zarr directories, misidentifying the format for pybids/ZarrNii and downstream tools. Derive the extension from the source asset instead: .ome.zarr/.zarr are kept as-is, while compound file extensions collapse to the plain format extension (x.ome.ims -> .ims), matching existing datasets. The sidecar path is now built via build_path with a .json extension so zarr assets get _SPIM.json rather than _SPIM.ome.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the symlink backend to correctly name BIDS symlinks and sidecars based on the true source asset format, enabling correct OME-Zarr packaging and avoiding misleading .ims outputs.
Changes:
- Derive the symlink extension from the source path (preserving
.ome.zarr/.zarr, collapsing compound file extensions like.ome.ims→.ims). - Build sidecar paths via
build_path(..., extension=".json")to avoid incorrect.ome.jsonnaming for compound extensions. - Add tests covering
.ome.zarrdirectory sources and.ome.imsfile sources.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/spimpack/backends/symlink.py | Derives BIDS link extension from the source and fixes sidecar path construction for compound extensions. |
| tests/test_writer.py | Adds coverage for .ome.zarr and .ome.ims extension handling and sidecar placement. |
Suppressed comments (2)
src/spimpack/backends/symlink.py:58
- When an existing output path is a real directory (not a symlink),
unlink()will raiseIsADirectoryError. With.ome.zarrsupport, it’s more likely users may already have a directory at the intended link path (e.g., from a previous copy-based run), which would cause the writer to crash instead of overwriting cleanly. Handle directories explicitly before unlinking.
if link_path.exists() or link_path.is_symlink():
link_path.unlink()
src/spimpack/backends/symlink.py:86
Path.suffixpreserves the original casing, so sources likeraw.OME.IMSwould produce a link ending in.IMS(previously this was always.ims). Since BIDS filenames and the existing writer behavior use lowercase extensions, normalize the returned suffix to lowercase to keep output stable.
return source.suffix or ".ims"
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| bids_entities["extension"] = _bids_extension(asset.spim_path) | ||
|
|
||
| rel_path = build_path(bids_entities, [BIDS_MICR_PATTERN]) | ||
| link_path = output_dir / rel_path | ||
| json_path = link_path.with_suffix(".json") | ||
| json_path = output_dir / build_path( | ||
| {**bids_entities, "extension": ".json"}, [BIDS_MICR_PATTERN] | ||
| ) |
There was a problem hiding this comment.
Good catch — updated the README in 12b8fcc: the symlink bullet now describes extension derivation (.ome.zarr/.zarr kept, compound file extensions collapsed to the plain format extension), and the path pattern ends in _SPIM{extension}.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Whoops, this was actually problematic for downstream SPIMquant analysis as path_templates expects consistent extensions, so mixed .ome.zarr/.ims extensions fail at config |
Summary
The symlink backend hardcoded
.imsas the link extension, so packaging an OME-Zarr source (documented as supported in the README) produced a link named_SPIM.imspointing at a zarr directory — silently misidentifying the format for pybids, ZarrNii, and downstream tools like spimquant. This never surfaced before because all existing datasets (e.g. the ki3 aggregate) only used.ims/.ome.imssources.Hit in practice while building
mouse_app_vaccine_aggregated, where batch1's only remaining sources are the SPIMprep OME-Zarrs.Changes
SymlinkWriternow derives the link extension from the source asset via a new_bids_extension()helper:.ome.zarr/.zarrstores keep their full extensionx.ome.ims→.ims), matching the naming in existing SPIMpack-generated datasetsbuild_pathusing a.jsonextension, so zarr assets get_SPIM.jsonrather than_SPIM.ome.json(previously derived viawith_suffix, which only replaces the last suffix).ome.zarrdirectory source and an.ome.imsfile sourceTesting
All writer/io/cli/validation tests pass (17 passed), and the change was exercised for real to build the 31-subject
mouse_app_vaccine_aggregateddataset (mixed.ims,.ome.ims, and.ome.zarrsources; all links verified).🤖 Generated with Claude Code