Repository navigation
FFT, Image: every method declares its arity, so a missing argument is MRI's error not a Go panic - #824
Merged
Conversation
tannevaled
force-pushed
the
fix/fft-image-arity
branch
from
October 18, 2026 13:18
912f54a to
258c94d
Compare
… MRI's error Closes #820. All 17 of FFT's methods and 14 of Image's indexed args[0..3] with no count check, so a missing argument surfaced as FFT.fft() ArgumentError: fft: runtime error: index out of range [0] with length 0 -- a Go panic recovered and relabelled, where MRI says "wrong number of arguments (given 0, expected 1)". Its class was accidental too: ArgumentError only because the recovery path labels every panic that way. NDArray was already clean because it uses ndArgc, which both now use. The counts go in the REGISTRATION SITE's signature rather than inside each closure: `def(name, min, max, fn)` for FFT and `d`/`s` for Image's instance and module methods. A check inside a body can be forgotten by the next method added; a parameter cannot. The three wrappers that forward to d (encode, unary, scalar) pass their own fixed arities, so the sixteen unary filters and the two scalar ones are covered by one line each rather than eighteen. Each arity comes from reading the body, not from a guess, and the ranges are the optional arguments a caller actually has: fftfreq/rfftfreq/psd 1..2 for numpy's sample spacing d, spectrogram 4..5, Image#set 5..6 for the alpha it defaults to 255. The test is a CENSUS, as #820 suggested, so a method added later is covered without being registered anywhere -- and it asserts the number WALKED (17, 3, 34) as well as the empty result, because a census that reads nothing otherwise looks exactly like a clean module. #save is held out of it, since it writes a file, and checked by name instead. Nineteen message rows pin MRI's wording for each shape -- fixed 1/2/3/4, both range forms, and the zero-argument form a caller trips by passing something. The count is only half of it: a reader rescuing ArgumentError and printing e.message has to be told what they passed wrongly, and "index out of range" does not tell them. A control for wrapping every method: eleven answers that were right before and have to stay so, including both optional arguments and #to_s, which is aliased from #inspect's native function and so inherits whatever wrapper #inspect got. Four negative controls, each checked to have really changed the file: dropping the check in FFT's def, dropping it in Image's d, taking fftfreq's optional spacing away, and taking Image#set's optional alpha away all turn the suite red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI's rbgo_core lane failed on the tests, not the fix: `NameError: uninitialized constant Image`. Image is the first of the gems the core build leaves out -- image.go and image_test.go have carried `//go:build !rbgo_core` all along -- so a test naming Image fails under that tag for a reason that has nothing to do with arity. The file is split along the line the repository already draws: wave71_arity_test.go FFT only, UNTAGGED -- FFT is in core wave71_arity_image_test.go Image only, !rbgo_core Checked under BOTH tags rather than one, which is the whole point of a split like this: 23 tests run on the default build (my six plus every pre-existing FFT, Image, canvas and images test) and 6 under -tags rbgo_core (the three FFT arity ones and the three pre-existing FFT ones), with Image correctly absent and no "no tests to run" -- an `ok` that ran nothing would look identical otherwise. The FFT half also gained the rows the split made room for: rfftfreq and psd's 1..2 ranges, fftn's fixed 2, and psd called both with and without its optional spacing, so every range form FFT has is exercised in both directions. go vet is clean on the default build, on -tags rbgo_core, and on GOOS=js GOARCH=wasm -tags rbgo_core; the core wasm module builds at 23_199_022 bytes, well under the 29_000_000 bound. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tannevaled
force-pushed
the
fix/fft-image-arity
branch
from
October 20, 2026 12:22
258c94d to
ad20f30
Compare
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 #820.
All 17 of
FFT's methods and 14 ofImage's indexedargs[0..3]with no count check, so a missing argument surfaced as a Go panic, recovered and relabelled. Its class was accidental too —ArgumentErroronly because the recovery path labels every panic that way.NDArraywas already clean because it usesndArgc, which both now use.The counts live in the signature, not in the body
def(name, min, max, fn)forFFT, andd/sforImage's instance and module methods. A check written inside a body can be forgotten by the next method someone adds; a parameter cannot. The three wrappers that forward tod—encode,unary,scalar— pass their own fixed arities, so the sixteen unary filters and the two scalar ones are covered by one line each instead of eighteen.Each arity comes from reading the body, and the ranges are the optional arguments a caller really has:
fftfreq/rfftfreq/psddspectrogramImage#setThe test is a census, and says how far it walked
A census covers a method added later without it being registered anywhere — which a list of names does not. And it asserts the number walked (
17 3 34) as well as the empty result, because a census that reads nothing otherwise looks exactly like a clean module.#saveis held out, since it writes a file, and checked by name instead.Nineteen message rows pin MRI's wording for each shape: fixed 1/2/3/4, both range forms, and the too-many-arguments form. The count is only half of it — someone rescuing
ArgumentErrorand printinge.messagehas to be told what they passed wrongly, and "index out of range" does not tell them.Controls
Eleven answers that were right before and have to stay so, since every method is now wrapped:
FFT.fft([1,2,3,4])→[10, -2, -2, -2],fftfreqwith and without spacing,hann(3), therfft/irfftround trip,Image#set/#getwith and without alpha, the filters' shapes,#to_png, and#to_s— which is aliased from#inspect's native function and so inherits whatever wrapper#inspectgot.Four negative controls, each verified to have really changed the file:
FFT'sdefImage'sdfftfreq's optional spacing awayImage#set's optional alpha awayThe existing
Image,FFTand canvas tests pass unchanged;gofmt,go vetandcredscanclean. Branched frommainso it is independent of #819, which found the defect but does not fix it.Measured 2026-10-18 with go1.27.2,
CGO_ENABLED=0.🤖 Generated with Claude Code