refactor: one class per augmentation, and explicit constructor parameters - #66
Open
Hendrik-code wants to merge 1 commit into
Open
refactor: one class per augmentation, and explicit constructor parameters#66Hendrik-code wants to merge 1 commit into
Hendrik-code wants to merge 1 commit into
Conversation
…ters
The GPU transforms dispatched on a config string: RandomConvTransformGPU took
kernel_type in {Laplace, Scharr, GaussianBlur, UnsharpMask, RandConv},
RandomFunctionGPU took an arbitrary func, RandomGammaGPU took invert_image. Five
augmentations behind one class name, twice over.
They are now one class each -- RandomLaplaceGPU, RandomScharrGPU,
RandomGaussianBlurGPU, RandomUnsharpMaskGPU, RandomRandConvGPU, RandomLog1pGPU,
RandomSqrtGPU, RandomSinGPU, RandomExpGPU, RandomSigmoidGPU, RandomGammaGPU and
RandomInvGammaGPU -- sharing a private base. This is what lets a config key be a
class name later in this series; until then it also means each augmentation can
be constructed, tested and discovered on its own. The standalone test sweep goes
from 199 to 223 subtests for that reason.
**kwargs is gone from every transform constructor. Kernel- and function-specific
parameters (absolute, sigma, unsharp_amount, kernel_sizes, mix_prob) were read
out of kwargs.get(), so they were invisible to inspect.signature and a typo in a
config silently selected the default. Their defaults here are the historical
kwargs.get() ones.
Removing **kwargs immediately surfaced a real defect, via mypy:
error: Unexpected keyword argument "crop" for "RandomLowResTransformGPU"
error: Unexpected keyword argument "crop" for "RandomAcqTransformGPU"
The two list pipelines passed crop= to both transforms; neither has ever accepted
it, and **kwargs swallowed it. 24 of the shipped configs set it. The sequential
pipeline never passed it, which is how the two drifted. Dropped.
Also RandomAffine3DCustom -> RandomAffineGPU and RandomPALETTEGPU ->
RandomPaletteGPU, and one_dim moves from a parameter of RandomAcqTransformGPU
into the class: all three pipelines passed one_dim=True literally, and the class
docstring already said "lower acquisition along one axes only", so the parameter
only ever offered a way to contradict the class.
The three `if` ladders still run, bridged by three aliases at the bottom of
contrast.py -- they read kernel_type and func from the config, which is the
dispatch these leaves exist to remove. They and the ladders go together later in
this series; the aliases are marked as temporary.
Verified byte-for-byte: the 24 seeded config digests are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 20, 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.
Stacked on #65. First of the registry-phase PRs — but it stands on its own, and no config changes.
Five augmentations behind one class name, twice over
The GPU transforms dispatched on a config string:
RandomConvTransformGPUkernel_type∈ {Laplace, Scharr, GaussianBlur, UnsharpMask, RandConv}RandomFunctionGPUfunccallableRandomGammaGPUinvert_imageThey're one class each now —
RandomLaplaceGPU,RandomScharrGPU,RandomGaussianBlurGPU,RandomUnsharpMaskGPU,RandomRandConvGPU,RandomLog1pGPU,RandomSqrtGPU,RandomSinGPU,RandomExpGPU,RandomSigmoidGPU,RandomGammaGPU,RandomInvGammaGPU— sharing a private base.This is what lets a config key be a class name later in the series. Independently of that, it means each augmentation can be constructed, tested and discovered on its own: the standalone test sweep goes from 199 to 223 subtests purely because there are now classes to discover.
**kwargsis gone (from the files this PR touches), and it was hiding a bugCorrection:
RandomDomainTransferGPUstill has**kwargsafter this PR —domain_transfer.pyis not touched here, because its other pending change (a hardcoded bank path) is unrelated to the class split. It is removed in the next PR, where registering it forces the issue.Kernel- and function-specific parameters (
absolute,sigma,unsharp_amount,kernel_sizes,mix_prob) were read out ofkwargs.get(). They were invisible toinspect.signature, so a typo in a config silently selected the default. Their defaults here are the historicalkwargs.get()ones.Removing
**kwargsimmediately surfaced a real defect — mypy found it, not me:The two list pipelines passed
crop=to both transforms. Neither has ever accepted it;**kwargsswallowed it. 24 of the shipped configs set it, and it has never done anything. The sequential pipeline never passed it, which is exactly how the two ladders drifted apart. Dropped.Renames, and one parameter removed
RandomAffine3DCustom→RandomAffineGPURandomPALETTEGPU→RandomPaletteGPUone_dimmoves from a parameter ofRandomAcqTransformGPUinto the class.On that last one: I checked whether it was a behaviour change and it is not. All three pipelines passed
one_dim=Trueliterally, and the class docstring already read "Randomly lower acquisition along one axes only" — so the parameter only ever offered a way to contradict the class. No config sets it.The ladders still run
The three
ifladders readkernel_typeandfuncfrom the config — precisely the dispatch these leaf classes exist to remove. Rather than rewrite them here and again when they're deleted, three aliases at the bottom ofcontrast.pybridge them, clearly marked as temporary. They and the ladders go together later in this series.Verification
All 24 seeded config digests are unchanged — same fixed volume, same seed, same output hash as before this PR. Full suite: 105 passed, 223 subtests.
ruffandmypyclean.