Repository navigation
Use ACPC-space atlases to create MIF files - #400
Merged
Merged
Conversation
- Deleted WarpConnectivityAtlases, _resample_atlas, label_convert, ExtractAtlasFiles (−242 lines net of the addition). - Added AtlasLUTs — the pure-Python LUT half of the old label_convert, unit-tested. - Drive-bys found while working there: SplitAtlasConfigs._run_interface was missing return runtime, and IFLOGGER was bound to 'nipype.interfaces', which isn't a valid nipype logger name and so evaluated to None. The second one surfaced because the new test exercised the warning path — every other module in interfaces/ uses 'nipype.interface'. qsirecon/workflows/recon/anatomical.py — four MapNodes as designed: conform_atlases → warp_atlases (ants.ApplyTransforms) and make_atlas_luts → convert_labels (nipype.interfaces.mrtrix3.LabelConvert, whose cmdline is byte-identical to the old subprocess call). Transforms now come from the template_to_acpc_xfm source node, matching odf_rois. The dead space/identity branch and the unconnected commands output are gone, and the two compute nodes now submit to the scheduler instead of running in the main process. Per your call, this stays behavior-preserving on #399: convert_labels.in_file is still the template-space atlas, with an XXX comment pointing at the issue. The fix is now a one-line connection change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #400 +/- ##
==========================================
- Coverage 50.47% 50.31% -0.16%
==========================================
Files 55 55
Lines 8139 8107 -32
Branches 944 943 -1
==========================================
- Hits 4108 4079 -29
+ Misses 3792 3789 -3
Partials 239 239 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors atlas preparation in the reconstruction anatomical workflow by removing the monolithic WarpConnectivityAtlases interface and replacing it with smaller, clearer workflow nodes, while extracting LUT generation into a pure-Python AtlasLUTs interface.
Changes:
- Replaces
WarpConnectivityAtlaseswith explicitConformAtlas→ants.ApplyTransforms→mrtrix3.LabelConvertnodes, and recombines outputs back intoatlas_configs. - Adds
AtlasLUTsinterface to generate MRtrix label lookup tables from atlas TSVs. - Adds unit tests for
AtlasLUTsand updates documentation references away fromWarpConnectivityAtlases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| qsirecon/workflows/recon/scalar_mapping.py | Updates docstring to reference the new atlas-preparation nodes in the anatomical workflow. |
| qsirecon/workflows/recon/anatomical.py | Refactors atlas preparation into separate MapNodes (conform, warp, LUT creation, label conversion) and recombines datasinked outputs into configs. |
| qsirecon/tests/test_interfaces.py | Adds unit tests covering AtlasLUTs outputs and 0-index handling. |
| qsirecon/interfaces/utils.py | Removes the old atlas-warping/labelconvert utilities and introduces the AtlasLUTs interface. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
Author
|
I'm going to move the relevant nodes into a new workflow and write unit tests to verify #399 before fixing that. |
tsalo
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #303. Closes #399.
Changes proposed:
WarpConnectivityAtlasesinterface into newinit_warp_atlases_wf.WarpConnectivityAtlases,_resample_atlas,label_convert,ExtractAtlasFiles.AtlasLUTs, which is the pure-Python LUT half of the oldlabel_convert.