search

peterbrittain / asciimatics

A cross platform package to do curses-like operations, plus higher level APIs and widgets to create text UIs and ASCII art animations
Apache License 2.0
3.61k stars 238 forks source link

small refactor to help create custom charts #344

Closed habermanUIUC closed 2 years ago

habermanUIUC commented 2 years ago

There's a lot of reusable functionality inside class _BarChartBase(DynamicRenderer): (charts.py)

that would be useful to create custom charts (not related to bar charts)

Describe the solution you'd like Can you

  1. Rename _BarChartBase to _BaseChart
  2. export _BaseChart in asciimatics/renderers/__init__.py
  3. Update the two classes inside chart.py to use _BaseChart

Describe alternatives you've considered Right now I just modify the source

peterbrittain commented 2 years ago

Let me flip that around for a sec... If you're creating new chart renderers, would you be interested in adding them to the package?

habermanUIUC commented 2 years ago

I'll be happy to contribute. My use case is a bit different than how the current HBar and VBar work. I pass in a data generator object that provides a bit more information to the Chart Renderer than the array of functions that is currently being passed in. It also pushes the attributes down to the data level.

For example, for a scatter plot, the plot container can manage things like axis, outlines, legends, and how to do blending for when multiple points share the same screen coordinates. But the data generator(s) are responsible for what character, color to use for the values.

One scatterplot can also manage different data streams as well if you wanted to plot different data sets on the same plot.

Rather than refactor how HBar and VBar work, it might make more sense to just add this thing that I am creating as a new Chart type. Perhaps refactor the common into _BaseChart and have different family of charts (bar, scatter, area, etc) subclass off the _BaseChart

BUT for now I do subclass off of _BaseChart.

peterbrittain commented 2 years ago

Sounds interesting... would be very happy to explore how to merge that into the project. Want to share some code and suggestions?

habermanUIUC commented 2 years ago

Yes. Let me refactor my stuff so I subclass off DynamicRenderer instead of _BarChartBase and we'll see how it goes.

peterbrittain commented 2 years ago

Cool. I'll close this and you can submit the PR when you're ready.