allow non-adaptive custom scale

[tcurdt]

I've set linechart.YAxisCustomScale(-20, 20) and I want the position of 0 to always stay at the same place.

Looking at the code, unfortunately the LineChart would still rescale the Y axis if a value is encountered that is outside of the range specified here

To me it seems adaptive should be an entirely separate option.

/ YAxisCustomScale when provided, the scale of the Y axis will be based on the
// specified minimum and maximum value instead of determining those from the
// LineChart series. Useful to visually stabilize the Y axis for LineChart
// applications that continuously feed values.
// The default behavior is to continuously determine the minimum and maximum
// value from the series before drawing the LineChart.
// Even when this option is provided, the LineChart would still rescale the Y
// axis if a value is encountered that is outside of the range specified here,
// i.e. smaller than the minimum or larger than the maximum.
// Both the minimum and the maximum must be valid numbers and the minimum must
// be smaller than the maximum.
//
// Providing this option also sets YAxisAdaptive.
func YAxisCustomScale(min, max float64) Option {
	return option(func(opts *options) {
		opts.yAxisCustomScale = &customScale{
			min: min,
			max: max,
		}
		opts.yAxisMode = axes.YScaleModeAdaptive
	})
}

[mum4k]

Agreed, in retrospect tying these two options together was ill-advised. We could try to address this by providing a new option YScaleModeAnchored that will set the Y axis mode back to Anchored. We would have to also add a unit test to confirm this actually works. The code is likely to crash since the current implementation might be depending on the fact that the axis adapts, so some minor changes may be needed deeper in the stack.

Open to other suggestions.

@tcurdt is this something you would be interested in contributing?

[tcurdt]

I am sure I could add the option. Just not sure about the changes need deeper in the stack.

[mum4k]

I would start by just adding the option and a test case that verifies how it behaves. If we are lucky, it will just work. If it won't, I can help dissect the code on the PR that is adding the option.

[tcurdt]

It seems there already is an YScaleModeAnchored on the axes - which just isn't exposed.

https://github.com/mum4k/termdash/blob/master/widgets/linechart/internal/axes/scale.go#L46

I know it would be a breaking change, but frankly speaking just

func YAxisCustomScale(min, max float64) Option {
	return option(func(opts *options) {
		opts.yAxisCustomScale = &customScale{
			min: min,
			max: max,
		}
		//opts.yAxisMode = axes.YScaleModeAdaptive
	})
}

would make so much more sense.

And it would just require to add YAxisAdaptive as another option to get back to the old behaviour.

[tcurdt]

Hm. Odd. Even this refuses to work. Still expands beyond the min/max.

func YAxisCustomScale(min, max float64) Option {
	return option(func(opts *options) {
		opts.yAxisCustomScale = &customScale{
			min: min,
			max: max,
		}
		//opts.yAxisMode = axes.YScaleModeAdaptive
		opts.yAxisMode = axes.YScaleModeAnchored
	})
}

[tcurdt]

Seems like this also need to change

https://github.com/mum4k/termdash/blob/master/widgets/linechart/linechart.go#L174

but then the draw panics

panic: container.Draw =>
error: unable to draw widget *linechart.LineChart:
failure for series first[9] on scale
YScale{Min:Value{Round(-5) => -5}, Max:Value{Round(5) => 5}, Step:Value{Round(0.12658227848101267) => 0.13}, GraphHeight:20}, yd.Scale.ValueToPixel(5.620833778521305) =>
position 82 out of bounds 0 <= pos <= 79

[mum4k]

Sorry it would be great if I could be more helpful, but I am afraid I forgot all the details of the code in question.

Setting a custom scale may need more code changes, because this either breaks some assumptions or simply triggers a bug when the scale is set manually.

I think you likely picked this up from the error message above - when the custom scale was set, the code attempted to place a point outside of the scale (out of bounds). We will have to track down the piece of code that determines the placement and debug it. What I would recommend is taking the very case from the error message above and converting it into a unit test.

[tcurdt]

This seems to be already covered by the existing tests:

    --- FAIL: TestLineChartDraws/custom_Y_scale,_negative_and_positive,_values_don't_fit_so_adjusted (0.00s)
        linechart_test.go:1867: Draw => unexpected error: failure for series first[0] on scale YScale{Min:Value{Round(-200) => -200}, Max:Value{Round(200) => 200}, Step:Value{Round(12.903225806451612) => 12.91}, GraphHeight:8}, yd.Scale.ValueToPixel(-400) => position -15 out of bounds 0 <= pos <= 31, wantDrawErr: false
FAIL

Here the test of course still expresses the wrong expectations.

[tcurdt]

That's with

diff --git a/widgets/linechart/linechart.go b/widgets/linechart/linechart.go
index 5880d45..47e670f 100644
--- a/widgets/linechart/linechart.go
+++ b/widgets/linechart/linechart.go
@@ -172,8 +172,9 @@ func (lc *LineChart) yMinMax() (float64, float64) {
 	}
 
 	if lc.opts.yAxisCustomScale != nil {
-		minimums = append(minimums, lc.opts.yAxisCustomScale.min)
-		maximums = append(maximums, lc.opts.yAxisCustomScale.max)
+		// minimums = append(minimums, lc.opts.yAxisCustomScale.min)
+		// maximums = append(maximums, lc.opts.yAxisCustomScale.max)
+		return lc.opts.yAxisCustomScale.min, lc.opts.yAxisCustomScale.max
 	}
 
 	min, _ := minMax(minimums)
diff --git a/widgets/linechart/options.go b/widgets/linechart/options.go
index 150f28b..c98336b 100644
--- a/widgets/linechart/options.go
+++ b/widgets/linechart/options.go
@@ -156,7 +156,8 @@ func YAxisCustomScale(min, max float64) Option {
 			min: min,
 			max: max,
 		}
-		opts.yAxisMode = axes.YScaleModeAdaptive
+		//opts.yAxisMode = axes.YScaleModeAdaptive
+		opts.yAxisMode = axes.YScaleModeAnchored
 	})
 }
 

[tcurdt]

I guess this something I could work around by limiting the input data - not great but a workaround.
If I find a way to disallow the zooming via mouse.

[mum4k]

@tcurdt I would like to help advise here, but I am having a hard time figuring out the current state of things. Would you mind summarizing the current behavior Vs. the desired one?

[tcurdt]

Alright - the summary is as follows:

• the assignment of opts.yAxisMode = axes.YScaleModeAdaptive should be just removed (unfortunately that does not make it just work)
• removing (or temporarily setting opts.yAxisMode = axes.YScaleModeAnchored) already does show up in the tests as error
• a work around is to enforce the min/max range before handing it to the widget
• the work around falls short when the users zooms or moves the graph (which hopefully can be disabled)

Does that make things clearer?

[mum4k]

I was more after the high level. I think what you commented describes how to adjust the existing test.

I am trying to understand what our goal is. Linechart already supports custom scale, but I think that doesn't work for your use case, because it still scales if a value outside of the range is encountered. Is our goal to work on a mode where that automatic scaling is disabled.

I am not sure how mouse zoom falls into the story. We can indeed add an option to disable zooming, but if we implement this, we need to also make sure that while enabled, mouse zooming works with the custom scale.

Do I understand this correctly?

[tcurdt]

Ah! OK, well, I think it would be nice to be able to disable zooming - but that's really for another ticket I guess.
The idea was just to limit the data myself. So if the user is not able to zoom (s)he wouldn't notice the difference.
It was just an idea of a quick work around.

My use case: I assume you know what an oscilloscope is? I want to implement something similar.
For this it's important that neither the position of 0 nor the scale changes automatically.
It needs to have the same visual reference all the time.
If values go beyond min/max they just leave the visible window - not repositioning or scaling it.

Does that make more sense now?

Frankly speaking it sounds so simple:
It's basically just plotting the data on the canvas with a fix scale.
Maybe the work around could be to write an entirely different more dumb widget instead?

That said, just writing my own widget would still leave it broken IMO.
I do think that opts.yAxisMode = axes.YScaleModeAdaptive should not be the default for YAxisCustomScale.
Which then requires fixing anyway.

WDYT is the best path forward?