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 <coconutruben@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1140027
Reviewed-by: Mengqi Guo <mqg@chromium.org>

3 files changed, 79 insertions, 51 deletions

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()