Skip to content

refactor: collapse eleven torchio wrappers into a table - #62

Open
Hendrik-code wants to merge 1 commit into
hm/dedup-kernelsfrom
hm/dedup-torchio
Open

refactor: collapse eleven torchio wrappers into a table#62
Hendrik-code wants to merge 1 commit into
hm/dedup-kernelsfrom
hm/dedup-torchio

Conversation

@Hendrik-code

Copy link
Copy Markdown
Collaborator

Stacked on #61.

cpu/artifact.py and cpu/spatial.py held eleven functions between them — aug_motion, aug_ghosting, aug_spike, aug_bias_field, aug_blur, aug_noise, aug_swap, aug_flip, aug_affine, aug_elastic, aug_anisotropy — that were the same seventeen lines each, differing only in which tio.Random* they constructed:

def aug_ghosting(img, seg):
    if img.shape[0] == 2:
        subject = tio.RandomGhosting()(tio.Subject(...))   # <- only this line differs
        ...

They become two tables and one apply_tio call. −270 / +31.

It had already drifted

aug_anisotropy's single-channel branch passed axis=0 to tio.LabelMap:

tio.Subject(image=tio.ScalarImage(tensor=img), seg=tio.LabelMap(tensor=seg, axis=0))

Nothing else did that, and torchio stores it as unrecognised metadata rather than acting on it. It was most likely meant to be tio.RandomAnisotropy(axes=...). Collapsing the copies drops it — a behaviour change only in the sense that a typo stops being passed along.

Notes for review

  • The tables hold factories, not instances, so each call still constructs a freshly seeded transform exactly as the functions did. There's a test for that.
  • get_parameters in both classes had the same five lines of random_pick logic written out separately; that's torchio_ops.select now.
  • apply_tio reads images out of the Subject by key rather than by attribute. torchio synthesises subject.image at runtime, which no type checker can see — the old code only passed mypy because those functions were unannotated and mypy skips unannotated bodies.

Testing

unit_tests/test_torchio_ops.py, 18 tests / 27 subtests, 8s.

It deliberately does not run all eleven transforms. apply_tio is the same code whichever transform it is handed, so running each would exercise torchio rather than this change — and I found out the hard way that RandomElasticDeformation(max_displacement=40) on a test-sized volume takes minutes, which would have put that straight into CI. Instead: every entry is checked for constructibility and freshness, the cheap ones run end to end, and apply_tio itself is tested against both input layouts (single-channel, and the two-channel "step 2" layout where channel 1 is an odd-disc segmentation) plus the property that LabelMap channels are never interpolated.

Full suite: 100 passed, 197 subtests. ruff and mypy clean.

cpu/artifact.py and cpu/spatial.py held eleven functions between them --
aug_motion, aug_ghosting, aug_spike, aug_bias_field, aug_blur, aug_noise,
aug_swap, aug_flip, aug_affine, aug_elastic, aug_anisotropy -- that were the same
seventeen lines each, differing only in which tio.Random* they constructed. They
are now entries in two tables and one apply_tio call: -270 lines, +31.

The eleven-fold copy had already drifted. aug_anisotropy's single-channel branch
passed axis=0 to tio.LabelMap, which nothing else did and which torchio stores as
unrecognised metadata rather than acting on. It was most likely meant to be
tio.RandomAnisotropy(axes=...). Collapsing the copies drops it, which is a
behaviour change only in the sense that a typo stops being passed.

The tables hold factories rather than instances, so each call still constructs a
freshly seeded transform exactly as the functions did.

get_parameters in both classes had the same five lines of random_pick logic
written out separately; that is torchio_ops.select now.

apply_tio reads images out of the Subject by key rather than by attribute. torchio
synthesises subject.image at runtime, which no type checker can see -- it only
passed mypy because these functions were unannotated and mypy skips unannotated
bodies.

unit_tests/test_torchio_ops.py covers the two input layouts every one of the
eleven had to handle, that the LabelMap channels are not interpolated, and
select's behaviour. It deliberately does not run all eleven transforms: apply_tio
is the same code whichever it is handed, so that would exercise torchio rather
than this change, and RandomElasticDeformation(max_displacement=40) on a
test-sized volume takes minutes. Each entry is checked for constructibility and
the cheap ones run end to end.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 20, 2026 12:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants