Repository: jonathanj/eliottree

Issue:
Human-readable values should only be formatted when rendered instead of modifying the tree data
The problem with modifying the tree data is that it makes it very difficult to write queries against it if eliot-tree is changing it in undisclosed ways to suit it's renderer.

Discussion:

Patch:
diff --git a/eliottree/_cli.py b/eliottree/_cli.py
index 6b0d24e..c333d8c 100644
--- a/eliottree/_cli.py
+++ b/eliottree/_cli.py
@@ -2,36 +2,20 @@ import argparse
 import codecs
 import json
 import sys
-from datetime import datetime
 from itertools import chain
 
 from six import PY3
 from six.moves import map
-from toolz import compose
 
 from eliottree import (
     Tree, filter_by_jmespath, filter_by_uuid, render_task_nodes)
 
 
-def _convert_timestamp(task):
-    """
-    Convert a ``timestamp`` key to a ``datetime``.
-    """
-    task['timestamp'] = datetime.utcfromtimestamp(task['timestamp'])
-    return task
-
-
-def build_task_nodes(files=None, select=None, task_uuid=None,
-                     human_readable=True):
+def build_task_nodes(files=None, select=None, task_uuid=None):
     """
     Build the task nodes given some input data, query criteria and formatting
     options.
     """
-    def task_transformers():
-        if human_readable:
-            yield _convert_timestamp
-        yield json.loads
-
     def filter_funcs():
         if select is not None:
             for query in select:
@@ -47,8 +31,7 @@ def build_task_nodes(files=None, select=None, task_uuid=None,
             files = [codecs.getreader('utf-8')(sys.stdin)]
 
     tree = Tree()
-    tasks = map(compose(*task_transformers()),
-                chain.from_iterable(files))
+    tasks = map(json.loads, chain.from_iterable(files))
     return tree.nodes(tree.merge_tasks(tasks, filter_funcs()))
 
 
@@ -65,13 +48,13 @@ def display_task_tree(args):
     nodes = build_task_nodes(
         files=args.files,
         select=args.select,
-        task_uuid=args.task_uuid,
-        human_readable=args.human_readable)
+        task_uuid=args.task_uuid)
     render_task_nodes(
         write=write,
         nodes=nodes,
         ignored_task_keys=set(args.ignored_task_keys) or None,
-        field_limit=args.field_limit)
+        field_limit=args.field_limit,
+        human_readable=args.human_readable)
 
 
 def main():
diff --git a/eliottree/render.py b/eliottree/render.py
index 4876a3e..626683c 100644
--- a/eliottree/render.py
+++ b/eliottree/render.py
@@ -8,20 +8,42 @@ DEFAULT_IGNORED_KEYS = set([
     u'message_type'])
 
 
-def _format_value(value):
+def _format_value_raw(value):
     """
-    Format a value for a task tree.
+    Format a value.
     """
     if isinstance(value, datetime):
         if PY3:
             return value.isoformat(' ')
         else:
             return value.isoformat(' ').decode('ascii')
-    elif isinstance(value, text_type):
+    return None
+
+
+def _format_value_hint(value, hint):
+    """
+    Format a value given a rendering hint.
+    """
+    if hint == u'timestamp':
+        return _format_value_raw(datetime.utcfromtimestamp(value))
+    return None
+
+
+def _format_value(value, field_hint=None, human_readable=False):
+    """
+    Format a value for a task tree.
+    """
+    if isinstance(value, text_type):
         return value
     elif isinstance(value, binary_type):
         # We guess bytes values are UTF-8.
         return value.decode('utf-8', 'replace')
+    if human_readable:
+        formatted = _format_value_raw(value)
+        if formatted is None:
+            formatted = _format_value_hint(value, field_hint)
+        if formatted is not None:
+            return formatted
     result = repr(value)
     if isinstance(result, binary_type):
         result = result.decode('utf-8', 'replace')
@@ -48,7 +70,7 @@ def _truncate_value(value, limit):
     return value
 
 
-def _render_task(write, task, ignored_task_keys, field_limit):
+def _render_task(write, task, ignored_task_keys, field_limit, human_readable):
     """
     Render a single ``_TaskNode`` as an ``ASCII`` tree.
 
@@ -64,6 +86,9 @@ def _render_task(write, task, ignored_task_keys, field_limit):
 
     :type ignored_task_keys: ``set`` of ``text_type``
     :param ignored_task_keys: Set of task key names to ignore.
+
+    :type human_readable: ``bool``
+    :param human_readable: Should this be rendered as human-readable?
     """
     _write = _indented_write(write)
     num_items = len(task)
@@ -78,9 +103,12 @@ def _render_task(write, task, ignored_task_keys, field_limit):
                 _render_task(write=_write,
                              task=value,
                              ignored_task_keys={},
-                             field_limit=field_limit)
+                             field_limit=field_limit,
+                             human_readable=human_readable)
             else:
-                _value = _format_value(value)
+                _value = _format_value(value,
+                                       field_hint=key,
+                                       human_readable=human_readable)
                 if field_limit:
                     first_line = _truncate_value(_value, field_limit)
                 else:
@@ -96,7 +124,8 @@ def _render_task(write, task, ignored_task_keys, field_limit):
                         _write(line + '\n')
 
 
-def _render_task_node(write, node, field_limit, ignored_task_keys):
+def _render_task_node(write, node, field_limit, ignored_task_keys,
+                      human_readable):
     """
     Render a single ``_TaskNode`` as an ``ASCII`` tree.
 
@@ -112,6 +141,9 @@ def _render_task_node(write, node, field_limit, ignored_task_keys):
 
     :type ignored_task_keys: ``set`` of ``text_type``
     :param ignored_task_keys: Set of task key names to ignore.
+
+    :type human_readable: ``bool``
+    :param human_readable: Should this be rendered as human-readable?
     """
     _child_write = _indented_write(write)
     write(
@@ -120,17 +152,20 @@ def _render_task_node(write, node, field_limit, ignored_task_keys):
         write=_child_write,
         task=node.task,
         field_limit=field_limit,
-        ignored_task_keys=ignored_task_keys)
+        ignored_task_keys=ignored_task_keys,
+        human_readable=human_readable)
 
     for child in node.children():
         _render_task_node(
             write=_child_write,
             node=child,
             field_limit=field_limit,
-            ignored_task_keys=ignored_task_keys)
+            ignored_task_keys=ignored_task_keys,
+            human_readable=human_readable)
 
 
-def render_task_nodes(write, nodes, field_limit, ignored_task_keys=None):
+def render_task_nodes(write, nodes, field_limit, ignored_task_keys=None,
+                      human_readable=False):
     """
     Render a tree of task nodes as an ``ASCII`` tree.
 
@@ -147,6 +182,9 @@ def render_task_nodes(write, nodes, field_limit, ignored_task_keys=None):
 
     :type ignored_task_keys: ``set`` of ``text_type``
     :param ignored_task_keys: Set of task key names to ignore.
+
+    :type human_readable: ``bool``
+    :param human_readable: Should this be rendered as human-readable?
     """
     if ignored_task_keys is None:
         ignored_task_keys = DEFAULT_IGNORED_KEYS
@@ -156,7 +194,8 @@ def render_task_nodes(write, nodes, field_limit, ignored_task_keys=None):
             write=write,
             node=node,
             field_limit=field_limit,
-            ignored_task_keys=ignored_task_keys)
+            ignored_task_keys=ignored_task_keys,
+            human_readable=human_readable)
         write('\n')
 
 
diff --git a/setup.py b/setup.py
index e54a48d..9767156 100644
--- a/setup.py
+++ b/setup.py
@@ -29,7 +29,6 @@ setup(
     install_requires=[
         "six>=1.9.0",
         "jmespath>=0.7.1",
-        "toolz>=0.7.2",
     ],
     extras_require={
         "dev": ["pytest>=2.7.1", "testtools>=1.8.0"],


Test patch:
diff --git a/eliottree/test/test_render.py b/eliottree/test/test_render.py
index 81d3128..81dea43 100644
--- a/eliottree/test/test_render.py
+++ b/eliottree/test/test_render.py
@@ -15,14 +15,14 @@ class FormatValueTests(TestCase):
     """
     Tests for ``eliottree.render._format_value``.
     """
-    def test_datetime(self):
+    def test_datetime_human_readable(self):
         """
         Format ``datetime`` values as ISO8601.
         """
         now = datetime(2015, 6, 6, 22, 57, 12)
         self.assertThat(
-            _format_value(now),
-            Equals('2015-06-06 22:57:12'))
+            _format_value(now, human_readable=True),
+            Equals(u'2015-06-06 22:57:12'))
 
     def test_unicode(self):
         """
@@ -59,6 +59,16 @@ class FormatValueTests(TestCase):
                 _format_value({'a': u('\N{SNOWMAN}')}),
                 Equals("{'a': u'\\u2603'}"))
 
+    def test_timestamp_hint(self):
+        """
+        Format "timestamp" hinted data as timestamps.
+        """
+        # datetime(2015, 6, 6, 22, 57, 12)
+        now = 1433631432
+        self.assertThat(
+            _format_value(now, field_hint='timestamp', human_readable=True),
+            Equals(u'2015-06-06 22:57:12'))
+
 
 class RenderTaskNodesTests(TestCase):
     """
@@ -85,6 +95,28 @@ class RenderTaskNodesTests(TestCase):
                 '    +-- app:action@2/succeeded\n'
                 '        `-- timestamp: 1425356800\n\n'))
 
+    def test_tasks_human_readable(self):
+        """
+        Render two tasks of sequential levels, by default most standard Eliot
+        task keys are ignored, values are formatted to be human readable.
+        """
+        fd = StringIO()
+        tree = Tree()
+        tree.merge_tasks([action_task, action_task_end])
+        render_task_nodes(
+            write=fd.write,
+            nodes=tree.nodes(),
+            field_limit=0,
+            human_readable=True)
+        self.assertThat(
+            fd.getvalue(),
+            Equals(
+                'f3a32bb3-ea6b-457c-aa99-08a3d0491ab4\n'
+                '+-- app:action@1/started\n'
+                '    `-- timestamp: 2015-03-03 04:26:40\n'
+                '    +-- app:action@2/succeeded\n'
+                '        `-- timestamp: 2015-03-03 04:26:40\n\n'))
+
     def test_multiline_field(self):
         """
         When no field limit is specified for task values, multiple lines are