Repository navigation
Improve orientation robustness in SH/DTI scripts - #1360
Merged
Merged
Conversation
…cripts to StatefulImage, requires the sh_basis/is_legacy overrides on create_from
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A moderate metadata-preservation issue remains, along with a minor fusion test coverage gap.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Updates five SH/DTI CLI scripts to use StatefulImage for orientation-aware processing, metadata handling, and stronger fusion validation.
Changes:
- Preserves source orientation during processing and output.
- Adds metadata, dtype, and header handling.
- Adds LAS/RAS regression coverage.
- Validates compatibility across fused inputs.
| File | Summary | Review note |
|---|---|---|
src/scilpy/io/tests/test_stateful_image.py |
Tests metadata, dtype, header, and file-map behavior. | — |
src/scilpy/io/stateful_image.py |
Enhances orientation-aware image and metadata handling. | Moderate (2 votes): preserve extra and file_map after reorientation. |
src/scilpy/cli/tests/test_sh_to_rish.py |
Adds LAS RISH coverage. | — |
src/scilpy/cli/tests/test_sh_to_aodf.py |
Adds LAS aODF coverage. | — |
src/scilpy/cli/tests/test_sh_fusion.py |
Adds LAS fusion and header validation coverage. | Nit (1 vote): add a third incompatible input. |
src/scilpy/cli/tests/test_sh_convert.py |
Adds LAS SH conversion coverage. | — |
src/scilpy/cli/tests/test_dti_convert_tensors.py |
Adds LAS tensor conversion coverage. | — |
src/scilpy/cli/scil_sh_to_rish.py |
Uses orientation-aware RISH processing and masks. | — |
src/scilpy/cli/scil_sh_to_aodf.py |
Uses orientation-aware aODF processing. | — |
src/scilpy/cli/scil_sh_fusion.py |
Reorients inputs and validates fusion headers. | — |
src/scilpy/cli/scil_sh_convert.py |
Uses orientation-aware SH conversion. | — |
src/scilpy/cli/scil_dti_convert_tensors.py |
Uses orientation-aware tensor conversion. | — |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev_3.0.x #1360 +/- ##
=============================================
- Coverage 72.95% 72.94% -0.01%
=============================================
Files 304 304
Lines 27053 27059 +6
Branches 3839 3840 +1
=============================================
+ Hits 19737 19739 +2
- Misses 5707 5710 +3
- Partials 1609 1610 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…it_branch_6_shdti
…o split_branch_6_shdti
frheault
changed the title
CHrlS98
reviewed
Oct 21, 2026
CHrlS98
left a comment
CHrlS98
left a comment
Contributor
There was a problem hiding this comment.
One small question for my understanding.
CHrlS98
approved these changes
Oct 30, 2026
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.
Quick description
Depends on PR 1 (
split_branch_1_core), included here so this PR's tests pass on their own.Updates 5 scripts (
scil_dti_convert_tensors,scil_sh_convert,scil_sh_fusion,scil_sh_to_aodf,scil_sh_to_rish) to scilpy's newer image-loading code. None of these are known to have produced wrong results before: their math doesn't depend on which orientation the file is stored in, so this is a consistency/robustness update, not a bug fix. As a bonus,scil_sh_fusionnow checks that all files being merged share a compatible header, not just the first two.I validated the before/after behavior using this data/code.
Type of change
Provide data, screenshots, command line to test (if relevant)
pytest src/scilpy/cli/tests/test_dti_convert_tensors.py src/scilpy/cli/tests/test_sh_convert.py src/scilpy/cli/tests/test_sh_fusion.py src/scilpy/cli/tests/test_sh_to_aodf.py src/scilpy/cli/tests/test_sh_to_rish.py src/scilpy/io/tests/test_stateful_image.py -v56 passed.
Checklist
🤖 Generated with Claude Code