From fa8c8e721af34cdbf091c3d4654b23259de1fd5b Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Wed, 15 Sep 2021 16:31:59 +0930 Subject: [PATCH 1/7] First pass --- esrally/reporter.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/esrally/reporter.py b/esrally/reporter.py index fc163464b..fff43b580 100644 --- a/esrally/reporter.py +++ b/esrally/reporter.py @@ -379,7 +379,7 @@ def _write_report(self, metrics_table, metrics_table_console): self.report_format, self.cwd, self.numbers_align, - headers=["Metric", "Task", "Baseline", "Contender", "Diff", "Unit"], + headers=["Metric", "Task", "Baseline", "Contender", "Diff", "Unit", "Diff %"], data_plain=metrics_table, data_rich=metrics_table_console, ) @@ -864,15 +864,23 @@ def _line(self, metric, baseline, contender, task, unit, treat_increase_as_impro formatter(contender), self._diff(baseline, contender, treat_increase_as_improvement, formatter), unit, + self._diff(baseline, contender, treat_increase_as_improvement, formatter, as_percentage=True) ] else: return [] - def _diff(self, baseline, contender, treat_increase_as_improvement, formatter=lambda x: x): + def _diff(self, baseline, contender, treat_increase_as_improvement, formatter=lambda x: x, as_percentage=False): def identity(x): return x - diff = formatter(contender - baseline) + def _safe_divide(n, d): + return n/d if d else 0 + + if as_percentage: + diff = formatter(_safe_divide((contender - baseline), baseline)*100) + else: + diff = formatter(contender - baseline) + if self.plain: color_greater = identity color_smaller = identity From 3bacec74e67dfb7533168af31d26713d91605d5a Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Thu, 16 Sep 2021 10:50:55 +0930 Subject: [PATCH 2/7] Second pass --- esrally/reporter.py | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/esrally/reporter.py b/esrally/reporter.py index fff43b580..e1bb3b3bf 100644 --- a/esrally/reporter.py +++ b/esrally/reporter.py @@ -64,7 +64,6 @@ def write_single_report(report_file, report_format, cwd, numbers_align, headers, formatter = format_as_csv else: raise exceptions.SystemSetupError("Unknown report format '%s'" % report_format) - print_internal(formatter(headers, data_rich)) if len(report_file) > 0: normalized_report_file = rio.normalize_path(report_file, cwd) @@ -876,11 +875,6 @@ def identity(x): def _safe_divide(n, d): return n/d if d else 0 - if as_percentage: - diff = formatter(_safe_divide((contender - baseline), baseline)*100) - else: - diff = formatter(contender - baseline) - if self.plain: color_greater = identity color_smaller = identity @@ -894,10 +888,22 @@ def _safe_divide(n, d): color_smaller = console.format.green color_neutral = console.format.neutral - if diff > 0: - return color_greater("+%.5f" % diff) - elif diff < 0: - return color_smaller("%.5f" % diff) + if as_percentage: + diff = contender - baseline + percentage_diff = round(formatter(abs(_safe_divide(diff, baseline)) * 100.0), 2) + # If the percentage difference cannot be rounded up to more than 0.00, then let's just color it neutral + if percentage_diff == 0.00: + return color_neutral("%.2f" % percentage_diff+"%") + elif diff > 0: + return color_greater("%.2f" % percentage_diff+"%") + elif diff < 0: + return color_smaller("-%.2f" % percentage_diff+"%") else: - # tabulate needs this to align all values correctly - return color_neutral("%.5f" % diff) + diff = formatter(contender - baseline) + if diff > 0: + return color_greater("+%.5f" % diff) + elif diff < 0: + return color_smaller("%.5f" % diff) + else: + # tabulate needs this to align all values correctly + return color_neutral("%.5f" % diff) From b34dbc509e55b669fb37c006c403e7d6994024fe Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Thu, 16 Sep 2021 17:43:49 +0930 Subject: [PATCH 3/7] Fix tests --- esrally/reporter.py | 2 +- tests/reporter_test.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/esrally/reporter.py b/esrally/reporter.py index e1bb3b3bf..02cb726a3 100644 --- a/esrally/reporter.py +++ b/esrally/reporter.py @@ -349,7 +349,7 @@ def report(self, r1, r2): metric_table_plain = self._metrics_table(baseline_stats, contender_stats, plain=True) metric_table_rich = self._metrics_table(baseline_stats, contender_stats, plain=False) # Writes metric_table_rich to console, writes metric_table_plain to file - self._write_report(metric_table_plain, metric_table_rich) + self._write_report(metrics_table=metric_table_plain, metrics_table_console=metric_table_rich) def _metrics_table(self, baseline_stats, contender_stats, plain): self.plain = plain diff --git a/tests/reporter_test.py b/tests/reporter_test.py index cf2e2c2b2..41c71e7ef 100644 --- a/tests/reporter_test.py +++ b/tests/reporter_test.py @@ -25,11 +25,11 @@ def setUp(self): self.empty_header = ["Header"] self.empty_data = [] - self.metrics_header = ["Metric", "Task", "Baseline", "Contender", "Diff", "Unit"] + self.metrics_header = ["Metric", "Task", "Baseline", "Contender", "Diff", "Unit", "Diff %"] self.metrics_data = [ - ["Min Throughput", "index", "17300", "18000", "700", "ops/s"], - ["Median Throughput", "index", "17500", "18500", "1000", "ops/s"], - ["Max Throughput", "index", "17700", "19000", "1300", "ops/s"], + ["Min Throughput", "index", "17300", "18000", "700", "ops/s", "4.04%"], + ["Median Throughput", "index", "17500", "18500", "1000", "ops/s", "5.71%"], + ["Max Throughput", "index", "17700", "19000", "1300", "ops/s", "7.34%"], ] self.numbers_align = "right" From 178dffd84e335e0ec3c2512703e4a45ca33faa47 Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Thu, 16 Sep 2021 17:44:38 +0930 Subject: [PATCH 4/7] Revert changes --- esrally/reporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esrally/reporter.py b/esrally/reporter.py index 02cb726a3..e1bb3b3bf 100644 --- a/esrally/reporter.py +++ b/esrally/reporter.py @@ -349,7 +349,7 @@ def report(self, r1, r2): metric_table_plain = self._metrics_table(baseline_stats, contender_stats, plain=True) metric_table_rich = self._metrics_table(baseline_stats, contender_stats, plain=False) # Writes metric_table_rich to console, writes metric_table_plain to file - self._write_report(metrics_table=metric_table_plain, metrics_table_console=metric_table_rich) + self._write_report(metric_table_plain, metric_table_rich) def _metrics_table(self, baseline_stats, contender_stats, plain): self.plain = plain From ecf53fb33a9337d5bdbb206913fe75102ceecf8f Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Thu, 16 Sep 2021 18:00:28 +0930 Subject: [PATCH 5/7] I should add linting on save --- esrally/reporter.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/esrally/reporter.py b/esrally/reporter.py index e1bb3b3bf..1614fd24b 100644 --- a/esrally/reporter.py +++ b/esrally/reporter.py @@ -863,7 +863,7 @@ def _line(self, metric, baseline, contender, task, unit, treat_increase_as_impro formatter(contender), self._diff(baseline, contender, treat_increase_as_improvement, formatter), unit, - self._diff(baseline, contender, treat_increase_as_improvement, formatter, as_percentage=True) + self._diff(baseline, contender, treat_increase_as_improvement, formatter, as_percentage=True), ] else: return [] @@ -873,7 +873,7 @@ def identity(x): return x def _safe_divide(n, d): - return n/d if d else 0 + return n / d if d else 0 if self.plain: color_greater = identity @@ -893,11 +893,11 @@ def _safe_divide(n, d): percentage_diff = round(formatter(abs(_safe_divide(diff, baseline)) * 100.0), 2) # If the percentage difference cannot be rounded up to more than 0.00, then let's just color it neutral if percentage_diff == 0.00: - return color_neutral("%.2f" % percentage_diff+"%") + return color_neutral("%.2f" % percentage_diff + "%") elif diff > 0: - return color_greater("%.2f" % percentage_diff+"%") + return color_greater("%.2f" % percentage_diff + "%") elif diff < 0: - return color_smaller("-%.2f" % percentage_diff+"%") + return color_smaller("-%.2f" % percentage_diff + "%") else: diff = formatter(contender - baseline) if diff > 0: From b3ec5141767a29cea2e746c4126e2d9847ac2860 Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Wed, 22 Sep 2021 11:05:29 +0930 Subject: [PATCH 6/7] Implement suggested changes --- esrally/reporter.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/esrally/reporter.py b/esrally/reporter.py index 1614fd24b..bd77774b4 100644 --- a/esrally/reporter.py +++ b/esrally/reporter.py @@ -889,21 +889,21 @@ def _safe_divide(n, d): color_neutral = console.format.neutral if as_percentage: - diff = contender - baseline - percentage_diff = round(formatter(abs(_safe_divide(diff, baseline)) * 100.0), 2) - # If the percentage difference cannot be rounded up to more than 0.00, then let's just color it neutral - if percentage_diff == 0.00: - return color_neutral("%.2f" % percentage_diff + "%") - elif diff > 0: - return color_greater("%.2f" % percentage_diff + "%") - elif diff < 0: - return color_smaller("-%.2f" % percentage_diff + "%") + diff = formatter(_safe_divide(contender - baseline, baseline) * 100.0) + precision = 2 + suffix = "%" else: diff = formatter(contender - baseline) - if diff > 0: - return color_greater("+%.5f" % diff) - elif diff < 0: - return color_smaller("%.5f" % diff) - else: - # tabulate needs this to align all values correctly - return color_neutral("%.5f" % diff) + precision = 5 + suffix = "" + + # ensures that numbers that appear as "zero" are also colored neutrally + threshold = 10**-precision + formatted = f"{diff:.{precision}f}{suffix}" + + if diff >= threshold: + return color_greater(f"+{formatted}") + elif diff <= -threshold: + return color_smaller(formatted) + else: + return color_neutral(formatted) \ No newline at end of file From d38a00fed97a70985749a9f9aee607f0c41cc90e Mon Sep 17 00:00:00 2001 From: Brad Deam Date: Wed, 22 Sep 2021 11:07:25 +0930 Subject: [PATCH 7/7] I should add lint and format on save --- esrally/reporter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esrally/reporter.py b/esrally/reporter.py index bd77774b4..0b9791063 100644 --- a/esrally/reporter.py +++ b/esrally/reporter.py @@ -898,7 +898,7 @@ def _safe_divide(n, d): suffix = "" # ensures that numbers that appear as "zero" are also colored neutrally - threshold = 10**-precision + threshold = 10 ** -precision formatted = f"{diff:.{precision}f}{suffix}" if diff >= threshold: @@ -906,4 +906,4 @@ def _safe_divide(n, d): elif diff <= -threshold: return color_smaller(formatted) else: - return color_neutral(formatted) \ No newline at end of file + return color_neutral(formatted)