refactor: collapse eleven torchio wrappers into a table - #62
Open
Hendrik-code wants to merge 1 commit into
Open
Conversation
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>
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.
Stacked on #61.
cpu/artifact.pyandcpu/spatial.pyheld 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 whichtio.Random*they constructed:They become two tables and one
apply_tiocall. −270 / +31.It had already drifted
aug_anisotropy's single-channel branch passedaxis=0totio.LabelMap: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
get_parametersin both classes had the same five lines ofrandom_picklogic written out separately; that'storchio_ops.selectnow.apply_tioreads images out of theSubjectby key rather than by attribute. torchio synthesisessubject.imageat 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_tiois 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 thatRandomElasticDeformation(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, andapply_tioitself 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 thatLabelMapchannels are never interpolated.Full suite: 100 passed, 197 subtests.
ruffandmypyclean.