Support None value for telemetry values.

tools/telemetry/telemetry/value/list_of_string_values.py

Index: tools/telemetry/telemetry/value/list_of_string_values.py
diff --git a/tools/telemetry/telemetry/value/list_of_string_values.py b/tools/telemetry/telemetry/value/list_of_string_values.py
index eaecbe1227f105435c5a394cb17b483ef64fbf4e..afce8fbaade703bf6d665a281612ea5cecda015f 100644
--- a/tools/telemetry/telemetry/value/list_of_string_values.py
+++ b/tools/telemetry/telemetry/value/list_of_string_values.py
@@ -3,19 +3,21 @@
 # found in the LICENSE file.
 

[nednguyen, 2014/08/15 14:06:52]

I think we should kill string & list_of_string_values. It's only used for
specifying trace's file path, which will soon replaced by TracedValue.
Really, we don't have any real world case of string value, and I can't come up
with one that string value is the only solution.

 from telemetry import value as value_module
+from telemetry.value import none_values
 
 
 class ListOfStringValues(value_module.Value):
   def __init__(self, page, name, units, values,
-               important=True, description=None,
+               important=True, description=None, none_value_reason=None,
                same_page_merge_policy=value_module.CONCATENATE):
     super(ListOfStringValues, self).__init__(page, name, units, important,
                                              description)
-    assert len(values) > 0
-    assert isinstance(values, list)
-    for v in values:
-      assert isinstance(v, basestring)
+    assert values is None or (len(values) > 0 and isinstance(values, list))
+    if isinstance(values, list):
+      assert all(isinstance(v, basestring) for v in values)

[chrishenry, 2014/08/15 04:43:20]

Maybe a little more readable:
if values is not None:
  assert len(values) > 0
  assert isinstance(values, list)
  assert all(isinstance(v, basestring) for v in values)

(similarly for the other class

[ariblue, 2014/08/15 22:10:26]

Good call. Done.

+    none_values.ValidateValue(values, none_value_reason)
     self.values = values
+    self.none_value_reason = none_value_reason
     self.same_page_merge_policy = same_page_merge_policy
 
   def __repr__(self):
@@ -62,6 +64,10 @@ class ListOfStringValues(value_module.Value):
   def AsDict(self):
     d = super(ListOfStringValues, self).AsDict()
     d['values'] = self.values
+
+    if self.none_value_reason is not None:
+      d['none_value_reason'] = self.none_value_reason
+
     return d
 
   @staticmethod
@@ -69,6 +75,9 @@ class ListOfStringValues(value_module.Value):
     kwargs = value_module.Value.GetConstructorKwArgs(value_dict, page_dict)
     kwargs['values'] = value_dict['values']
 
+    if 'none_value_reason' in value_dict:
+      kwargs['none_value_reason'] = value_dict['none_value_reason']
+
     return ListOfStringValues(**kwargs)
 
   @classmethod
@@ -81,32 +90,34 @@ class ListOfStringValues(value_module.Value):
           v0.page, v0.name, v0.units,
           values[0].values,
           important=v0.important,
-          same_page_merge_policy=v0.same_page_merge_policy)
+          same_page_merge_policy=v0.same_page_merge_policy,
+          none_value_reason=v0.none_value_reason)
 
     assert v0.same_page_merge_policy == value_module.CONCATENATE
-    all_values = []
-    for v in values:
-      all_values.extend(v.values)
-    return ListOfStringValues(
-        v0.page, v0.name, v0.units,
-        all_values,
-        important=v0.important,
-        same_page_merge_policy=v0.same_page_merge_policy)
+    return cls._MergeLikeValues(values, v0.page, v0.name)
 
   @classmethod
   def MergeLikeValuesFromDifferentPages(cls, values,
                                         group_by_name_suffix=False):
     assert len(values) > 0
     v0 = values[0]
+    name = v0.name_suffix if group_by_name_suffix else v0.name
+    return cls._MergeLikeValues(values, None, name)
+
+  @classmethod
+  def _MergeLikeValues(cls, values, page, name):
+    v0 = values[0]
     all_values = []
+    none_value_reason = None
     for v in values:
+      if(v.values is None):

[chrishenry, 2014/08/15 04:43:20]

Remove parenthesis around conditional

[ariblue, 2014/08/15 22:10:26]

Done.

+        all_values = None
+        none_value_reason = none_values.MERGE_FAILURE_REASON
+        break
       all_values.extend(v.values)

[chrishenry, 2014/08/15 04:43:20]

Let's rename all_values to merged_values? Then None would make a bit more sense.

[ariblue, 2014/08/15 22:10:26]

Done.

-    if not group_by_name_suffix:
-      name = v0.name
-    else:
-      name = v0.name_suffix
     return ListOfStringValues(
-        None, name, v0.units,
+        page, name, v0.units,
         all_values,
         important=v0.important,
-        same_page_merge_policy=v0.same_page_merge_policy)
+        same_page_merge_policy=v0.same_page_merge_policy,
+        none_value_reason=none_value_reason)