From 9b4909334c5b2531e9dd08d81ca00f51ddd1a906 Mon Sep 17 00:00:00 2001 From: Ruben Rodriguez Buchillon Date: Wed, 25 Jul 2018 09:20:19 +0800 Subject: stats_manager: replace prefixes with flags in StatsManager StatsManager right now uses a '__' prefix to indicate that sample_msecs should be the first key in the summary. Additionally, there is a NOSHOW_PREFIX to potentially hide rails in the summary. This CL replaces that approach with constructor arguments to feed into the StatsManager what rails to hide, and the order to print a summary. This results in cleaner code, and less information leakage across classes as hiding & sorting becomes an implementation detail. It also adds two new unit tests to StatsManagerTest to verify this behavior works as intended. As a nit this CL also replaces AddValue with AddSample as Sample is more descriptive for the use-cases. BRANCH=None BUG=chromium:760267 TEST=manual testing, same output as before, and unittests still pass Change-Id: I52ca0d85c4600691fce8d4c74fd2a81fc4aa440f Signed-off-by: Ruben Rodriguez Buchillon Reviewed-on: https://chromium-review.googlesource.com/1140027 Reviewed-by: Mengqi Guo --- extra/usb_power/powerlog.py | 2 +- extra/usb_power/stats_manager.py | 66 ++++++++++++++++--------------- extra/usb_power/stats_manager_unittest.py | 62 ++++++++++++++++++++--------- 3 files changed, 79 insertions(+), 51 deletions(-) (limited to 'extra') diff --git a/extra/usb_power/powerlog.py b/extra/usb_power/powerlog.py index a431b05b5f..0bceb23f06 100755 --- a/extra/usb_power/powerlog.py +++ b/extra/usb_power/powerlog.py @@ -754,7 +754,7 @@ class powerlog(object): value = aggregate_record[name] * multiplier csv += ", %.2f" % value name_type = name[0] + Spower.INA_SUFFIX[name[1]] - self._data.AddValue(name_type, value) + self._data.AddSample(name_type, value) else: csv += ", " csv += ", %d" % aggregate_record["status"] diff --git a/extra/usb_power/stats_manager.py b/extra/usb_power/stats_manager.py index 43ff6eee2d..92d15098bc 100644 --- a/extra/usb_power/stats_manager.py +++ b/extra/usb_power/stats_manager.py @@ -14,11 +14,6 @@ import os import numpy STATS_PREFIX = '@@' -# used to aid sorting of dict keys -KEY_PREFIX = '__' -# This prefix is used for keys that should not be shown in the summary tab, such -# as timeline keys. -NOSHOW_PREFIX = '!!' LONG_UNIT = { '': 'N/A', @@ -41,15 +36,15 @@ class StatsManager(object): Example usage: >>> stats = StatsManager() - >>> stats.AddValue(TIME_KEY, 50.0) - >>> stats.AddValue(TIME_KEY, 25.0) - >>> stats.AddValue(TIME_KEY, 40.0) - >>> stats.AddValue(TIME_KEY, 10.0) - >>> stats.AddValue(TIME_KEY, 10.0) - >>> stats.AddValue('frobnicate', 11.5) - >>> stats.AddValue('frobnicate', 9.0) - >>> stats.AddValue('foobar', 11111.0) - >>> stats.AddValue('foobar', 22222.0) + >>> stats.AddSample(TIME_KEY, 50.0) + >>> stats.AddSample(TIME_KEY, 25.0) + >>> stats.AddSample(TIME_KEY, 40.0) + >>> stats.AddSample(TIME_KEY, 10.0) + >>> stats.AddSample(TIME_KEY, 10.0) + >>> stats.AddSample('frobnicate', 11.5) + >>> stats.AddSample('frobnicate', 9.0) + >>> stats.AddSample('foobar', 11111.0) + >>> stats.AddSample('foobar', 22222.0) >>> stats.CalculateStats() >>> print(stats.SummaryToString()) @@ NAME COUNT MEAN STDDEV MAX MIN @@ -60,6 +55,9 @@ class StatsManager(object): Attributes: _data: dict of list of readings for each domain(key) _unit: dict of unit for each domain(key) + _order: list of formatting order for domains. Domains not listed are + displayed in sorted order + _hide_domains: collection of domains to hide when formatting summary string _summary: dict of stats per domain (key): min, max, count, mean, stddev _logger = StatsManager logger @@ -68,27 +66,30 @@ class StatsManager(object): CalculateStats() is called. """ - def __init__(self): + # pylint: disable=W0102 + def __init__(self, order=[], hide_domains=[]): """Initialize infrastructure for data and their statistics.""" self._data = collections.defaultdict(list) self._unit = collections.defaultdict(str) + self._order = order + self._hide_domains = hide_domains self._summary = {} self._logger = logging.getLogger('StatsManager') - def AddValue(self, domain, value): - """Add one value for a domain. + def AddSample(self, domain, sample): + """Add one sample for a domain. Args: - domain: the domain name for the value. - value: one time reading for domain, expect type float. + domain: the domain name for the sample. + sample: one time sample for domain, expect type float. """ - if isinstance(value, int): - value = float(value) - if isinstance(value, float): - self._data[domain].append(value) + if isinstance(sample, int): + sample = float(sample) + if isinstance(sample, float): + self._data[domain].append(sample) return - self._logger.warn('value %s for domain %s is not a number, thus ignored.', - value, domain) + self._logger.warn('sample %s for domain %s is not a number, thus ignored.', + sample, domain) def SetUnit(self, domain, unit): """Set the unit for a domain. @@ -134,13 +135,16 @@ class StatsManager(object): """ headers = ('NAME', 'COUNT', 'MEAN', 'STDDEV', 'MAX', 'MIN') table = [headers] - for domain in sorted(self._summary.keys()): - if domain.startswith(NOSHOW_PREFIX): - continue + # determine what domains to display & and the order + domains_to_display = set(self._summary.keys()) - set(self._hide_domains) + display_order = [key for key in self._order if key in domains_to_display] + domains_to_display -= set(display_order) + display_order.extend(sorted(domains_to_display)) + for domain in display_order: stats = self._summary[domain] if not domain.endswith(self._unit[domain]): domain = '%s_%s' % (domain, self._unit[domain]) - row = [domain.lstrip(KEY_PREFIX)] + row = [domain] row.append(str(stats['count'])) for entry in headers[2:]: row.append('%.2f' % stats[entry.lower()]) @@ -188,8 +192,6 @@ class StatsManager(object): """ data = {} for domain in self._summary: - if domain.startswith(NOSHOW_PREFIX): - continue unit = LONG_UNIT.get(self._unit[domain], self._unit[domain]) data_entry = {'mean': self._summary[domain]['mean'], 'unit': unit} data[domain] = data_entry @@ -221,4 +223,4 @@ class StatsManager(object): fname = '%s.txt' % domain fname = os.path.join(dirname, fname) with open(fname, 'w') as f: - f.write('\n'.join('%.2f' % value for value in data) + '\n') + f.write('\n'.join('%.2f' % sample for sample in data) + '\n') diff --git a/extra/usb_power/stats_manager_unittest.py b/extra/usb_power/stats_manager_unittest.py index ed317eca02..3122fda455 100644 --- a/extra/usb_power/stats_manager_unittest.py +++ b/extra/usb_power/stats_manager_unittest.py @@ -7,6 +7,7 @@ from __future__ import print_function import json import os +import re import shutil import tempfile import unittest @@ -22,20 +23,20 @@ class TestStatsManager(unittest.TestCase): def _populate_dummy_stats(self): """Create a populated & processed StatsManager to test data retrieval.""" - self.data.AddValue('A', 99999.5) - self.data.AddValue('A', 100000.5) - self.data.AddValue('A', 'ERROR') + self.data.AddSample('A', 99999.5) + self.data.AddSample('A', 100000.5) + self.data.AddSample('A', 'ERROR') self.data.SetUnit('A', 'uW') self.data.SetUnit('A', 'mW') - self.data.AddValue('B', 1.5) - self.data.AddValue('B', 2.5) - self.data.AddValue('B', 3.5) + self.data.AddSample('B', 1.5) + self.data.AddSample('B', 2.5) + self.data.AddSample('B', 3.5) self.data.SetUnit('B', 'mV') self.data.CalculateStats() def _populate_dummy_stats_no_unit(self): - self.data.AddValue('B', 1000) - self.data.AddValue('A', 200) + self.data.AddSample('B', 1000) + self.data.AddSample('A', 200) self.data.SetUnit('A', 'blue') def setUp(self): @@ -47,33 +48,33 @@ class TestStatsManager(unittest.TestCase): """Delete the temporary directory and its content.""" shutil.rmtree(self.tempdir) - def test_AddValue(self): - """Adding a value successfully adds a value.""" - self.data.AddValue('Test', 1000) + def test_AddSample(self): + """Adding a sample successfully adds a sample.""" + self.data.AddSample('Test', 1000) self.data.SetUnit('Test', 'test') self.data.CalculateStats() summary = self.data.GetSummary() self.assertEqual(1, summary['Test']['count']) - def test_AddValueNoFloat(self): + def test_AddSampleNoFloat(self): """Adding a non number gets ignored and doesn't raise an exception.""" - self.data.AddValue('Test', 17) - self.data.AddValue('Test', 'fiesta') + self.data.AddSample('Test', 17) + self.data.AddSample('Test', 'fiesta') self.data.SetUnit('Test', 'test') self.data.CalculateStats() summary = self.data.GetSummary() self.assertEqual(1, summary['Test']['count']) - def test_AddValueNoUnit(self): + def test_AddSampleNoUnit(self): """Not adding a unit does not cause an exception on CalculateStats().""" - self.data.AddValue('Test', 17) + self.data.AddSample('Test', 17) self.data.CalculateStats() summary = self.data.GetSummary() self.assertEqual(1, summary['Test']['count']) def test_UnitSuffix(self): """Unit gets appended as a suffix in the displayed summary.""" - self.data.AddValue('test', 250) + self.data.AddSample('test', 250) self.data.SetUnit('test', 'mw') self.data.CalculateStats() summary_str = self.data.SummaryToString() @@ -81,7 +82,7 @@ class TestStatsManager(unittest.TestCase): def test_DoubleUnitSuffix(self): """If domain already ends in unit, verify that unit doesn't get appended.""" - self.data.AddValue('test_mw', 250) + self.data.AddSample('test_mw', 250) self.data.SetUnit('test_mw', 'mw') self.data.CalculateStats() summary_str = self.data.SummaryToString() @@ -136,6 +137,31 @@ class TestStatsManager(unittest.TestCase): # Verify nothing gets appended to domain for filename if no unit exists. self.assertIn('B.txt', files) + def test_SummaryToStringHideDomains(self): + """Keys indicated in hide_domains are not printed in the summary.""" + data = stats_manager.StatsManager(hide_domains=['A-domain']) + data.AddSample('A-domain', 17) + data.AddSample('B-domain', 17) + data.CalculateStats() + summary_str = data.SummaryToString() + self.assertIn('B-domain', summary_str) + self.assertNotIn('A-domain', summary_str) + + def test_SummaryToStringOrder(self): + """Order passed into StatsManager is honoured when formatting summary.""" + # StatsManager that should print D & B first, and the subsequent elements + # are sorted. + d_b_a_c_regexp = re.compile('D-domain.*B-domain.*A-domain.*C-domain', + re.DOTALL) + data = stats_manager.StatsManager(order=['D-domain', 'B-domain']) + data.AddSample('A-domain', 17) + data.AddSample('B-domain', 17) + data.AddSample('C-domain', 17) + data.AddSample('D-domain', 17) + data.CalculateStats() + summary_str = data.SummaryToString() + self.assertRegexpMatches(summary_str, d_b_a_c_regexp) + def test_SaveSummary(self): """SaveSummary properly dumps the summary into a file.""" self._populate_dummy_stats() -- cgit v1.2.1