Skip to content

Commit 87a84b1

Browse files
cinarclaude
andcommitted
Trim comments that narrated the bug fix instead of the code
Code comments should explain non-obvious behavior of the code itself; the bug/fix narrative belongs in the commit message and PR description, not inline, where it goes stale as the code evolves. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 77a7622 commit 87a84b1

2 files changed

Lines changed: 7 additions & 30 deletions

File tree

momentum/fisher.go

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -59,21 +59,11 @@ func NewFisher[T helper.Float]() *Fisher[T] {
5959

6060
// ComputeWithContext function takes a channel of numbers and computes the Fisher Transform.
6161
func (f *Fisher[T]) ComputeWithContext(ctx context.Context, closings <-chan T) <-chan T {
62-
// Split closings into three branches for Min, Max, and the aligned
63-
// closings below. This used to collect the whole input into a slice
64-
// upfront instead (to hand each branch its own independent channel),
65-
// but that made ComputeWithContext block synchronously until closings
66-
// closed before returning anything -- unlike every other indicator in
67-
// this package -- which deadlocks a caller that duplicates closings
68-
// itself and only starts draining the other branch after this call
69-
// returns.
7062
inputs := helper.DuplicateWithContext(ctx, closings, 3)
7163
input1, input2, input3 := inputs[0], inputs[1], inputs[2]
7264

73-
// Compute min and max. minValues feeds both the range (max - min) and
74-
// close-min-min below, so it needs its own duplicated copy for the
75-
// second use -- a channel only has one consumer's worth of values to
76-
// give out.
65+
// minValues is used twice below (range and close-minus-min), so it
66+
// needs its own duplicated copy for the second use.
7767
minSplice := helper.DuplicateWithContext(ctx, f.Min.ComputeWithContext(ctx, input1), 2)
7868
maxValues := f.Max.ComputeWithContext(ctx, input2)
7969

@@ -111,12 +101,9 @@ func (f *Fisher[T]) ComputeWithContext(ctx context.Context, closings <-chan T) <
111101

112102
// IdlePeriod is the initial period that Fisher Transform won't yield any results.
113103
func (f *Fisher[T]) IdlePeriod() int {
114-
// minValues, maxValues, and alignedClosings are all computed
115-
// independently over the same window of the original input (Min and
116-
// Max each skip their own Period-1 internally; alignedClosings skips
117-
// Period-1 to land on the same window's last close), not chained one
118-
// after another, so their delays don't compound: the combined idle
119-
// period is just Period-1, the same as Min/Max's own.
104+
// Min, Max, and the aligned closings are each independently delayed
105+
// by Period-1 from the same input, not chained one after another, so
106+
// the delay doesn't compound.
120107
return f.Period - 1
121108
}
122109

momentum/fisher_test.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,8 @@ func TestFisherSimple(t *testing.T) {
3838
t.Logf("Fisher values: %v", resultSlice[:5])
3939
}
4040

41-
// referenceFisher independently computes the Fisher Transform with a plain
42-
// sliding-window min/max over a slice, to verify Fisher's values rather
43-
// than just its output length or well-formedness.
41+
// referenceFisher computes the Fisher Transform with a plain sliding-window
42+
// min/max over a slice, independently of Fisher's channel implementation.
4443
func referenceFisher(closings []float64, period int) []float64 {
4544
var out []float64
4645

@@ -71,15 +70,6 @@ func referenceFisher(closings []float64, period int) []float64 {
7170
return out
7271
}
7372

74-
// TestFisherValues guards against a regression where minValues was read by
75-
// two consumers (the range and the close-minus-min terms) without being
76-
// duplicated first, and IdlePeriod() double-counted a delay that doesn't
77-
// compound (min, max, and the aligned closings are each independently
78-
// delayed by the same Period-1, not chained one after another the way
79-
// T3's EMAs are). Both silently produced wrong output rather than a build
80-
// or panic, so this compares against an independently computed reference
81-
// instead of just length or well-formedness, the way TestFisherSimple
82-
// already did.
8373
func TestFisherValues(t *testing.T) {
8474
closings := make([]float64, 400)
8575
for i := range closings {

0 commit comments

Comments
 (0)