From 65d5cfb4b7fcd996c7a495ac117a876c895a1ad7 Mon Sep 17 00:00:00 2001 From: Thorben Roemer Date: Wed, 25 Feb 2026 18:20:52 +0100 Subject: [PATCH 1/5] Use format_value consistently * use format_value across all chart types * format_value now takes the full args object instead of individual parameters. * format_value evaluates the args format, percentage, suffix and no_values * cvt_to_readable is no longer responsible for percentage conversion * suffix is no longer appended when no_values is True Signed-off-by: Thorben Roemer --- termgraph/chart.py | 77 ++++++++++++++++++++-------------------------- termgraph/utils.py | 6 +--- 2 files changed, 35 insertions(+), 48 deletions(-) diff --git a/termgraph/chart.py b/termgraph/chart.py index ae19130..4bd7fcf 100644 --- a/termgraph/chart.py +++ b/termgraph/chart.py @@ -15,35 +15,52 @@ just_fix_windows_console() def format_value( - value: Union[int, float], format_str_arg, percentage_arg, suffix_arg + value: Union[int, float], args ) -> str: """Format a value consistently across chart types.""" # Handle type conversions and defaults + format_str_arg = args.get_arg("format") if format_str_arg is None or not isinstance(format_str_arg, str): format_str = "{:<5.2f}" else: format_str = format_str_arg + percentage_arg = args.get_arg("percentage") if percentage_arg is None or not isinstance(percentage_arg, bool): percentage = False else: percentage = percentage_arg + suffix_arg = args.get_arg("suffix") if suffix_arg is None or not isinstance(suffix_arg, str): suffix = "" else: suffix = suffix_arg + no_values_arg = args.get_arg("no_values") + if no_values_arg is None or not isinstance(no_values_arg, bool): + no_values = False + else: + no_values = no_values_arg + + # Initial format formatted_val = format_str.format(value) - if percentage and "%" not in formatted_val: + if no_values is True: + return "" + elif percentage and "%" not in formatted_val: + return "" + elif args.get_arg("percentage") and "%" not in formatted_val: try: # Convert to percentage - numeric_value = float(formatted_val) - formatted_val = f"{numeric_value * 100:.0f}%" + numeric_value = float(value) + formatted_val = format_str.format(numeric_value * 100) + "%" except ValueError: # If conversion fails, just add % suffix formatted_val += "%" + else: + val, deg = cvt_to_readable(value) + formatted_val = f"{format_str.format(val)}{deg}" return f" {formatted_val}{suffix}" @@ -250,29 +267,14 @@ def draw(self) -> None: len_label = len(label) label = " " * len_label - if self.args.get_arg("label_before"): - fmt = "{}{}{}" - - else: - fmt = " {}{}{}" - - if self.args.get_arg("no_values"): - tail = self.args.get_arg("suffix") + tail = format_value( + values[j], + self.args + ) - else: - val, deg = cvt_to_readable( - values[j], self.args.get_arg("percentage") - ) - format_str = self.args.get_arg("format") - if isinstance(format_str, str): - formatted_val = format_str.format(val) - else: - formatted_val = f"{val:<5.2f}" # Default format - tail = fmt.format( - formatted_val, - deg, - self.args.get_arg("suffix"), - ) + if self.args.get_arg("label_before"): + # remove leading " " from format_value + tail = tail.lstrip() if colors and isinstance(colors, list) and j < len(colors): color = colors[j] @@ -354,15 +356,9 @@ def draw(self) -> None: tick=tick, ) - if self.args.get_arg("no_values"): - # Hide the values. - tail = "" - else: tail = format_value( sum(values), - self.args.get_arg("format"), - self.args.get_arg("percentage"), - self.args.get_arg("suffix"), + self.args ) print(tail) @@ -381,7 +377,7 @@ def __init__(self, data: Data, args: Args = Args()): def _prepare_vertical(self, value: float, num_blocks: int): """Prepare the vertical graph data.""" - self.value_list.append(str(value)) + self.value_list.append(format_value(value, self.args)) if self.maxi < num_blocks: self.maxi = num_blocks @@ -549,13 +545,8 @@ def draw(self) -> None: tick=tick, ) - if self.args.get_arg("no_values"): - tail = "" - else: - tail = format_value( - count_list[i][0], - self.args.get_arg("format"), - self.args.get_arg("percentage"), - self.args.get_arg("suffix"), - ) + tail = format_value( + count_list[i][0], + self.args + ) print(tail) diff --git a/termgraph/utils.py b/termgraph/utils.py index 600e0ab..56203b7 100644 --- a/termgraph/utils.py +++ b/termgraph/utils.py @@ -6,7 +6,7 @@ from .constants import UNITS, TICK, SM_TICK -def cvt_to_readable(num, percentage=False): +def cvt_to_readable(num): """Return the number in a human readable format. Examples: @@ -14,10 +14,6 @@ def cvt_to_readable(num, percentage=False): 12550 -> (12.55, 'K') 19561100 -> (19.561, 'M') """ - - if percentage: - return (num * 100, '%') - if num >= 1 or num <= -1: neg = num < 0 num = abs(num) From 4c0cba554151263cd9cf2039d6c8e645546a3cad Mon Sep 17 00:00:00 2001 From: Thorben Roemer Date: Wed, 25 Feb 2026 18:21:16 +0100 Subject: [PATCH 2/5] Fix --no-readable argument Support for the --no-readable argument was (accidentally?) removed in 41adfd4 Additional refactoring and consolidation (#120). With the refactored version of format_value, this is now an easy fix. Only call cvt_to_readable if no_readable is False (as it was before). It now also works for all chart types. Fixes 41adfd4 Signed-off-by: Thorben Roemer --- termgraph/chart.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/termgraph/chart.py b/termgraph/chart.py index 4bd7fcf..76cfc52 100644 --- a/termgraph/chart.py +++ b/termgraph/chart.py @@ -37,6 +37,12 @@ def format_value( else: suffix = suffix_arg + no_readable_arg = args.get_arg("no_readable") + if no_readable_arg is None or not isinstance(no_readable_arg, bool): + no_readable = False + else: + no_readable = no_readable_arg + no_values_arg = args.get_arg("no_values") if no_values_arg is None or not isinstance(no_values_arg, bool): no_values = False @@ -58,7 +64,7 @@ def format_value( except ValueError: # If conversion fails, just add % suffix formatted_val += "%" - else: + elif no_readable is False: val, deg = cvt_to_readable(value) formatted_val = f"{format_str.format(val)}{deg}" From 1711e4959ff66698cc59271e506db41674411aac Mon Sep 17 00:00:00 2001 From: Thorben Roemer Date: Wed, 25 Feb 2026 19:19:46 +0100 Subject: [PATCH 3/5] Skip unnecessary argument checks Since Args self-initializes with a default, I see no reason to do excessive type checking. Signed-off-by: Thorben Roemer --- termgraph/chart.py | 38 ++++---------------------------------- 1 file changed, 4 insertions(+), 34 deletions(-) diff --git a/termgraph/chart.py b/termgraph/chart.py index 76cfc52..4387117 100644 --- a/termgraph/chart.py +++ b/termgraph/chart.py @@ -18,43 +18,12 @@ def format_value( value: Union[int, float], args ) -> str: """Format a value consistently across chart types.""" - # Handle type conversions and defaults - format_str_arg = args.get_arg("format") - if format_str_arg is None or not isinstance(format_str_arg, str): - format_str = "{:<5.2f}" - else: - format_str = format_str_arg - - percentage_arg = args.get_arg("percentage") - if percentage_arg is None or not isinstance(percentage_arg, bool): - percentage = False - else: - percentage = percentage_arg - - suffix_arg = args.get_arg("suffix") - if suffix_arg is None or not isinstance(suffix_arg, str): - suffix = "" - else: - suffix = suffix_arg - - no_readable_arg = args.get_arg("no_readable") - if no_readable_arg is None or not isinstance(no_readable_arg, bool): - no_readable = False - else: - no_readable = no_readable_arg - - no_values_arg = args.get_arg("no_values") - if no_values_arg is None or not isinstance(no_values_arg, bool): - no_values = False - else: - no_values = no_values_arg + format_str = args.get_arg("format") # Initial format formatted_val = format_str.format(value) - if no_values is True: - return "" - elif percentage and "%" not in formatted_val: + if args.get_arg("no_values") is True: return "" elif args.get_arg("percentage") and "%" not in formatted_val: try: @@ -64,10 +33,11 @@ def format_value( except ValueError: # If conversion fails, just add % suffix formatted_val += "%" - elif no_readable is False: + elif args.get_arg("no_readable") is False: val, deg = cvt_to_readable(value) formatted_val = f"{format_str.format(val)}{deg}" + suffix = args.get_arg("suffix") return f" {formatted_val}{suffix}" From 0039e8a0317ba545ca4e10b2105c8378f46c4e76 Mon Sep 17 00:00:00 2001 From: Thorben Roemer Date: Wed, 25 Feb 2026 20:06:44 +0100 Subject: [PATCH 4/5] Add formatting tests Add tests that ensure formatting options are working as expected (and so that future refactorings don't break things again). A new helper function run_for_all_charts() is introduced. Histograms are optional, as their value/formatting works a bit differently. Signed-off-by: Thorben Roemer --- tests/test_charts.py | 183 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 176 insertions(+), 7 deletions(-) diff --git a/tests/test_charts.py b/tests/test_charts.py index 9704623..1b9618d 100644 --- a/tests/test_charts.py +++ b/tests/test_charts.py @@ -1,25 +1,25 @@ from termgraph.data import Data from termgraph.args import Args -from termgraph.chart import BarChart, VerticalChart +from termgraph.chart import BarChart, StackedChart, VerticalChart def test_barchart_draws_correctly(): labels = ["2007", "2008", "2009", "2010", "2011", "2012", "2014"] data_values = [[183.32], [231.23], [16.43], [50.21], [508.97], [212.05], [1.0]] - + data = Data(data_values, labels) args = Args(width=50, no_labels=False, suffix="", no_values=False) - + chart = BarChart(data, args) - + # Capture the output of the draw method import io from contextlib import redirect_stdout - + f = io.StringIO() with redirect_stdout(f): chart.draw() output = f.getvalue() - + # Assert that the output contains the expected elements assert "2007: " in output assert "183.32" in output @@ -53,7 +53,6 @@ def test_verticalchart_draws_correctly(): def test_custom_tick_appears_in_output(): labels = ["A", "B"] data_values = [[10], [20]] - data = Data(data_values, labels) args = Args(custom_tick="😀") @@ -69,3 +68,173 @@ def test_custom_tick_appears_in_output(): # Assert that the custom tick appears in the output assert "😀" in output + +def run_for_all_charts(func): + """ Runs a test function with all chart types, HistogramChart is optional as data format works differently there""" + for chart in [BarChart, StackedChart, VerticalChart]: + print(f"running {func.__name__} for {chart.__name__}") + func(chart) + +def test_format_default(): + labels = ["A", "B"] + data_values = [[10000], [20000]] + data = Data(data_values, labels) + args = Args() + + def test(ChartType): + chart = ChartType(data, args) + + import io + from contextlib import redirect_stdout + + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # values are default formatted and converted to readable + assert "10.00K" in output + assert "20.00K" in output + + run_for_all_charts(test) + +def test_format_no_readable(): + labels = ["A", "B"] + data_values = [[10000], [20000]] + data = Data(data_values, labels) + args = Args(no_readable=True) + + def test(ChartType): + chart = ChartType(data, args) + + import io + from contextlib import redirect_stdout + + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # values are not converted to readable format + assert "10000" in output + assert "20000" in output + assert "10.00K" not in output + assert "20.00K" not in output + + run_for_all_charts(test) + +def test_format_percentage(): + labels = ["A", "B"] + data_values = [[0.1], [0.275]] + data = Data(data_values, labels) + args = Args(percentage=True) + + def test(ChartType): + chart = ChartType(data, args) + + import io + from contextlib import redirect_stdout + + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # values are converted to percentage + assert "10.00%" in output + assert "27.50%" in output + + run_for_all_charts(test) + +def test_format_custom_format(): + labels = ["A", "B"] + data_values = [[10.375], [400.00000]] + data = Data(data_values, labels) + args = Args(format="{:3.1f}") + + def test(ChartType): + chart = ChartType(data, args) + + import io + from contextlib import redirect_stdout + + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # values are formatted with custom format + assert "10.4" in output + assert "400.0" in output + + run_for_all_charts(test) + +def test_format_no_values(): + labels = ["A", "B"] + data_values = [[10], [20]] + data = Data(data_values, labels) + args = Args(no_values=True) + + def test(ChartType): + chart = ChartType(data, args) + + import io + from contextlib import redirect_stdout + + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # values are not printed at all + assert "10" not in output + assert "20" not in output + + run_for_all_charts(test) + +def test_format_suffix(): + labels = ["A", "B"] + data_values = [[10], [20]] + data = Data(data_values, labels) + args = Args(suffix="suf") + + def test(ChartType): + chart = ChartType(data, args) + + import io + from contextlib import redirect_stdout + + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # values are suffixed + assert "10.00suf" in output + assert "20.00suf" in output + + run_for_all_charts(test) + +def test_no_values_with_suffix(): + labels = ["A", "B"] + data_values = [[10], [20]] + data = Data(data_values, labels) + args = Args(no_values=True, suffix="suf") + + def test(ChartType): + chart = ChartType(data, args) + + import io + from contextlib import redirect_stdout + + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # suffix not printed with no_values + assert "suf" not in output + assert "10" not in output + assert "20" not in output + + run_for_all_charts(test) \ No newline at end of file From fb17c6ad68dcdcd4b4fe061e7e22ead7fd767846 Mon Sep 17 00:00:00 2001 From: Thorben Roemer Date: Wed, 25 Feb 2026 20:09:32 +0100 Subject: [PATCH 5/5] Add custom tick support for VerticalChart When changing test_custom_tick_appears_in_output() to use the new run_for_all_tests() function, I discovered that VerticalChart had no support for custom ticks. After a little refactoring (tick is now a member of Chart and only initialized once), all Charts now support custom ticks. Signed-off-by: Thorben Roemer --- termgraph/chart.py | 28 +++++++++------------------- tests/test_charts.py | 32 ++++++++++++++++++++------------ 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/termgraph/chart.py b/termgraph/chart.py index 4387117..5adba1a 100644 --- a/termgraph/chart.py +++ b/termgraph/chart.py @@ -68,6 +68,10 @@ def __init__(self, data: Data, args: Args): self.args = args self.normal_data = self._normalize() + # Get custom tick if set, otherwise use default TICK + custom_tick = self.args.get_arg("custom_tick") + self.tick = custom_tick if isinstance(custom_tick, str) and custom_tick else TICK + def draw(self) -> None: """Draw the chart with the given data""" @@ -76,9 +80,6 @@ def draw(self) -> None: def _print_header(self) -> None: title = self.args.get_arg("title") - custom_tick = self.args.get_arg("custom_tick") - tick = custom_tick if isinstance(custom_tick, str) and custom_tick else TICK - if title is not None: print("") print(f"# {title}\n") @@ -91,7 +92,7 @@ def _print_header(self) -> None: if colors is not None and isinstance(colors, list): sys.stdout.write(f"\033[{colors[i]}m") # Start to write colorized. - sys.stdout.write(f"{tick} {self.data.categories[i]} ") + sys.stdout.write(f"{self.tick} {self.data.categories[i]} ") if colors: sys.stdout.write("\033[0m") # Back to original. @@ -140,9 +141,6 @@ def print_row( if doprint: print(label, tail, " ", end="") - # Get custom tick if set, otherwise use default TICK - custom_tick = self.args.get_arg("custom_tick") - tick = custom_tick if isinstance(custom_tick, str) and custom_tick else TICK print_row_core( value=float(value), @@ -150,7 +148,7 @@ def print_row( val_min=float(val_min), color=color, zero_as_small_tick=bool(self.args.get_arg("label_before")), - tick=tick, + tick=self.tick, ) if doprint: @@ -303,10 +301,6 @@ def draw(self) -> None: val_min = self.data.find_min() normal_data = self._normalize() - # Get custom tick if set, otherwise use default TICK - custom_tick = self.args.get_arg("custom_tick") - tick = custom_tick if isinstance(custom_tick, str) and custom_tick else TICK - for i in range(len(self.data.labels)): if self.args.get_arg("no_labels"): # Hide the labels. @@ -329,7 +323,7 @@ def draw(self) -> None: val_min=val_min, color=colors[j] if j < len(colors) else None, zero_as_small_tick=False, - tick=tick, + tick=self.tick, ) tail = format_value( @@ -359,7 +353,7 @@ def _prepare_vertical(self, value: float, num_blocks: int): self.maxi = num_blocks if num_blocks > 0: - self.vertical_list.append((TICK * num_blocks)) + self.vertical_list.append((self.tick * num_blocks)) else: self.vertical_list.append(SM_TICK) @@ -497,10 +491,6 @@ def draw(self) -> None: temp_data = Data(count_list, [f"bin_{i}" for i in range(len(count_list))]) normal_counts = temp_data.normalize(width) - # Get custom tick if set, otherwise use default TICK - custom_tick = self.args.get_arg("custom_tick") - tick = custom_tick if isinstance(custom_tick, str) and custom_tick else TICK - for i, (start_border, end_border) in enumerate(zip(borders[:-1], borders[1:])): if colors and colors[0]: color = colors[0] @@ -518,7 +508,7 @@ def draw(self) -> None: val_min=0, # Histogram always starts from 0 color=color, zero_as_small_tick=False, - tick=tick, + tick=self.tick, ) tail = format_value( diff --git a/tests/test_charts.py b/tests/test_charts.py index 1b9618d..d220999 100644 --- a/tests/test_charts.py +++ b/tests/test_charts.py @@ -1,6 +1,6 @@ from termgraph.data import Data from termgraph.args import Args -from termgraph.chart import BarChart, StackedChart, VerticalChart +from termgraph.chart import BarChart, StackedChart, VerticalChart, HistogramChart def test_barchart_draws_correctly(): labels = ["2007", "2008", "2009", "2010", "2011", "2012", "2014"] @@ -56,22 +56,30 @@ def test_custom_tick_appears_in_output(): data = Data(data_values, labels) args = Args(custom_tick="😀") - chart = BarChart(data, args) + def test(ChartType): + chart = ChartType(data, args) - import io - from contextlib import redirect_stdout + import io + from contextlib import redirect_stdout - f = io.StringIO() - with redirect_stdout(f): - chart.draw() - output = f.getvalue() + f = io.StringIO() + with redirect_stdout(f): + chart.draw() + output = f.getvalue() + + # Assert that the custom tick appears in the output + assert "😀" in output - # Assert that the custom tick appears in the output - assert "😀" in output + run_for_all_charts(test, with_histo=True) -def run_for_all_charts(func): +def run_for_all_charts(func, with_histo=False): """ Runs a test function with all chart types, HistogramChart is optional as data format works differently there""" - for chart in [BarChart, StackedChart, VerticalChart]: + charts = [BarChart, StackedChart, VerticalChart] + + if with_histo: + charts.append(HistogramChart) + + for chart in charts: print(f"running {func.__name__} for {chart.__name__}") func(chart)