Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the user to request an unscaled X axis on the LineChart #123

Closed
keithknott26 opened this issue Feb 14, 2019 · 13 comments
Closed

Allow the user to request an unscaled X axis on the LineChart #123

keithknott26 opened this issue Feb 14, 2019 · 13 comments
Labels
enhancement New feature or request
Milestone

Comments

@keithknott26
Copy link

Hello,

[Category: Question]

As I mentioned before - fantastic work so far! One thing I found might be missing would be a way to get the width of the lineChart for when you implement scrolling behavior.

I see some functions under Container which might be helpful, is there a way to return the effective width of the linechart itself (the piece where braille gets drawn to the screen [the canvas])? Right now I use a hacky method where the ui.TerminalDimensions() function is called from the termUI library to take the 85 percent of the total terminal width (because the linechart container takes up 85 percent plus another 15 percent for the text box). I'd like to remove the termUI dependency entirely and somehow find the width of the canvas.

termWidth, termHeight := ui.TerminalDimensions()
termWidth = int((float64(termWidth)*float64(0.85)))

Thanks!
Keith

@keithknott26 keithknott26 changed the title Textbox flickers when line chart is redrawn Question: A way to get the canvas size of the lineChart Feb 14, 2019
@mum4k
Copy link
Owner

mum4k commented Feb 15, 2019

Thank you very much for your support @keithknott26. Please don't hesitate to open feature requests for stuff you would find useful. It will be best for the project when it gets shaped around customer use cases.

We can easily expose the terminal size or the size of the linechart graph. Sounds like you need the latter so we probably want to do that inside the linechart widget itself. We can give you an exact size so there is no need for the 85% calculation.

Before we do that, can you help me understand what do you need the size for? I just want to make sure we have an opportunity to solve the right problem, rather than a "Y problem" (ref from http://xyproblem.info/).

What will you use the size for? I see that the original title was about flickering - is that related? Basically would you mind explaining the problem that you will be able to solve once you have the size?

@mum4k mum4k added the question Further information is requested label Feb 15, 2019
@keithknott26
Copy link
Author

keithknott26 commented Feb 15, 2019

Sure - Forgive the confusion I ended up sorting the flickering problem with the Text boxes, turns out the t.Reset() needed to be called closer to the t.Write() function so there was a minor delay in updating the text box content after the reset, resulting in screen flickering.

So I use the width of the line chart to determine how many data points I need in order to fill the canvas, so that when I "scroll" the X values series to the left there are is no unused portions of the canvas.
I only display a small portion of the complete series at any one time, so assuming the width of the line chart is 100 I would retrieve the LAST 100 data elements from a circular ring buffer and use those values to update the series giving the perception of scrolling from right to left when in reality only the last 100 records and labels are being displayed and new records are being added in behind the old ones. Records are added at an interval to the ring buffer in a different function. When there are no more records to add to the Buffer , "scrolling" stops. If I were to underestimate the size, the scrolling would take place only on a fraction of the screen: [----- ] vs [-------]

I'd really like to remove the first two lines, keep in mind the termHeight isn't actually used anywhere.

Code example:

//retrieve termWidth from termUI library
termWidth, termHeight := ui.TerminalDimensions()
//set termWidth to actual width of linechart (85% of screen)
termWidth = int((float64(termWidth) * float64(0.85)))

lc := linechart.New(
		linechart.AxesCellOpts(cell.FgColor(cell.ColorNumber(GraphAxes))),
		linechart.YLabelCellOpts(cell.FgColor(cell.ColorNumber(GraphYLabels))),
		linechart.XLabelCellOpts(cell.FgColor(cell.ColorNumber(GraphXLabels))),
	)

go periodic(ctx, time.Duration(100*time.Millisecond), func() error {
		var inputs []float64
		var inputLabels []string

                 //retrieve lineChart width num of elements from the data buffer
		inputs = buffer.Last(termHeight, termWidth) 

                 //retrieve lineChart width num of elements from the data labels buffer
		inputLabels = buffer.LastLabels(termHeight, termWidth)
		var labelMap = map[int]string{}
		for i, x := range inputLabels {
			labelMap[i] = x
		}
               if err := lc.Series("first", inputs,
			linechart.SeriesCellOpts(cell.FgColor(cell.ColorNumber(GraphLineOne))),
			linechart.SeriesXLabels(labelMap),
		); err != nil {
			return err
		}

		return nil
	})
	return lc
}  

Maybe you know a better way to accomplish the scrolling behavior? If there were a function to append records to the end of a series and then shift the contents of the canvas left I'd use that instead. Right now, if you add records to the end of the series it will compress the linegraph continually until all of the data fits (as it should). Effectively if you push one value to the beginning, one value is removed off the end making it nice for streaming realtime stats - it becomes hard to see small variances in data when the graph gets "too compressed"[the series has a high count of values].

@mum4k
Copy link
Owner

mum4k commented Feb 15, 2019

Thanks again @keithknott26, I know writing detailed response like this takes a lot of time. This helps a lot however, much appreciated.

Regarding the text widget - I believe we should add the ability to atomically Reset+Write - tracking this in #124.

As for the linechart, I understand the problem now. You probably know this - the line charts actually fits two values per each cell of width, because it uses the braille canvas (each cell is two pixels wide):

│● ●│ 0

We could expose a method that would report the LineChart's capacity so the caller knows how many values are needed to "fill" the linechart, but that would be similarly racy as the problem you encountered regarding the Reset+Write on the text widget. The caller might determine the capacity, then a terminal resize would happen, then the caller fills an incorrect number of values.

Therefore I am more inclined towards providing this functionality on the LineChart itself. The simplest way would be to add a new Option:

type Option interface {

Say an option defined like this:

// UnscaledX when provided, stops the LineChart from rescaling the X axis when it
// can't fit all the values, instead the LineCharts only displays the last n values that
// fit into its width. This results in hiding some values from the beginning of the
// series that didn't fit completely.
// The default behavior is to rescale the X axis to display all the values.
// This option takes no effect if all the values on the series fit into the
// LineChart area.
func UnscaledX() Option { ... }

Now this has the obvious problem that the caller won't have any idea how many values should be maintained in the values slice and it probably isn't practical to let the values slice just grow forever. For that purpose we could add a LineChart method as follows.

// ValueCapacity returns the number of values that can be fit onto the X axis without
// a need to rescale the axis. This is essentially the number of available pixels based
// on the current width of the LineChart.
// Note that this value changes each time the terminal resizes so there is
// no guarantee this remains the same next time Draw is called. Should be used as a
// hint only.
func (lc *LineChart) ValueCapacity() int { ... }

Please let me know what you think.

@keithknott26
Copy link
Author

This takes care of both the graph scaling when the slice is being populated with data and the size/capacity function. Perfect! This is exactly what I had in mind thank you.

@mum4k
Copy link
Owner

mum4k commented Feb 16, 2019

Great, thanks for confirming. I will look into implementing this shortly.

@mum4k mum4k added enhancement New feature or request and removed question Further information is requested labels Feb 16, 2019
@mum4k mum4k added this to the Release 0.7.0 milestone Feb 16, 2019
@mum4k mum4k changed the title Question: A way to get the canvas size of the lineChart Allow the user to request an unscaled X axis on the LineChart Feb 16, 2019
@mum4k
Copy link
Owner

mum4k commented Feb 16, 2019

This will be pushed to the devel branch shortly. I would appreciate if you could test it and report any issues.

@keithknott26
Copy link
Author

@mum4k - Im having trouble figuring out how to use the Options of the line chart. I was trying something like this:

lc := linechart.New(linechart.
linechart.AxesCellOpts(cell.FgColor(cell.ColorNumber(GraphAxes))),
linechart.YLabelCellOpts(cell.FgColor(cell.ColorNumber(GraphYLabels))),
linechart.XLabelCellOpts(cell.FgColor(cell.ColorNumber(GraphXLabels))),
linechart.YAxisCustomScale(0, 100),
)

I must be doing it wrong and not using the Options interface correctly

@mum4k
Copy link
Owner

mum4k commented Feb 16, 2019

Considering the context of this issue, you probably want the new X axis option:

func XAxisUnscaled() Option {

If that doesn't do what you wanted, can you help me by describing the expected behavior and the one observed?

@keithknott26
Copy link
Author

@mum4k ,

I finally had a chance to test this out with a large dataset and it looks like it's scrolling as expected! When the X-Axis fills up with enough values it starts removing values ( and the labels) from the front of the graph.

I've included a screenshot showing both scenarios (XAxisUnscaled on Top, w/out that option on Bottom). In the top graph you can observe the labels/data scrolling by while the bottom one continues to compress its data onto the X-Axis.:

image

Also I've confirmed that the vertical labels display correctly when the XaxisUnscaled option is used.

Feature confirmed, Nice one!

@mum4k
Copy link
Owner

mum4k commented Feb 18, 2019

That looks really good @keithknott26, thank you again for taking the time, testing the features and confirming.

Out of curiosity - how are you aligning the text in the two containers containing the statistics? If that is a plain Text widget - this is going to become easier for you once we will have the planned Table widget mentioned #5 implemented.

@keithknott26
Copy link
Author

keithknott26 commented Feb 18, 2019

@mum4k - I'm using a mixture of tabs and spaces, it works well enough as long as the numbers don't grow too large.

I was thinking of raising an enhancement to add the ability to draw a data average line across the linechart canvas. It can be achieved outside the linechart as well by adding another series and changing the line color - or is functionality something you'd prefer to keep outside of the linechart code? For streaming data in particular it would be helpful to see an average line (of all values) when the graph has scrolled left. I'm planning to add that feature next and thought I'd mention it.

Thanks again for all your hard work, if anything else comes to mind I'll be sure to run it by you.

edit: Here is a screenshot of the average line implemented:
image

@mum4k
Copy link
Owner

mum4k commented Feb 18, 2019

My first instinct is to conclude that this is something that doesn't need to be on the LineChart widget itself. As you noted - given that the caller controls the data, it is trivial to calculate this outside. So I am not sure what value we would add by implementing it here.

Please do open a feature request if you will encounter any issues implementing this, in which case we can look into enhancing the widget.

@keithknott26
Copy link
Author

I tend to agree, given how easy it was to implement outside the Linechart it doesn't make much sense to include it in the lower level code.

Happy to hear you added support for zooming I'll test that next!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants