Repository: airspeed-velocity/asv

Issue:
asv compare should use the unit attribute for displaying
I have a benchmark that counts number of objects with a `track_` method, but `asv run` outputs minutes, hours, days.

Discussion:

Patch:
diff --git a/asv/commands/compare.py b/asv/commands/compare.py
index 1096249..164c2ed 100644
--- a/asv/commands/compare.py
+++ b/asv/commands/compare.py
@@ -7,6 +7,7 @@ from __future__ import (absolute_import, division, print_function,
 import itertools
 
 from . import Command
+from ..benchmarks import Benchmarks
 from ..machine import iter_machine_files
 from ..results import iter_results_for_machine_and_hash
 from ..repo import get_repo, NoSuchNameError
@@ -181,6 +182,9 @@ class Compare(Command):
         stats_2 = {}
         versions_1 = {}
         versions_2 = {}
+        units = {}
+
+        benchmarks = Benchmarks.load(conf)
 
         if commit_names is None:
             commit_names = {}
@@ -210,6 +214,7 @@ class Compare(Command):
             machine_env_name = "{}/{}".format(machine, env_name)
             machine_env_names.add(machine_env_name)
             for name, value, stats in unroll_result(key, params, value, stats):
+                units[(name, machine_env_name)] = benchmarks.get(key, {}).get('unit')
                 results_1[(name, machine_env_name)] = value
                 stats_1[(name, machine_env_name)] = stats
                 versions_1[(name, machine_env_name)] = version
@@ -218,6 +223,7 @@ class Compare(Command):
             machine_env_name = "{}/{}".format(machine, env_name)
             machine_env_names.add(machine_env_name)
             for name, value, stats in unroll_result(key, params, value, stats):
+                units[(name, machine_env_name)] = benchmarks.get(key, {}).get('unit')
                 results_2[(name, machine_env_name)] = value
                 stats_2[(name, machine_env_name)] = stats
                 versions_2[(name, machine_env_name)] = version
@@ -300,7 +306,7 @@ class Compare(Command):
                 improved = True
             elif time_1 is None and time_2 is None:
                 # both failed
-                color = 'red'
+                color = 'default'
                 mark = ' '
             elif _isna(time_1) or _isna(time_2):
                 # either one was skipped
@@ -330,10 +336,12 @@ class Compare(Command):
             if only_changed and mark in (' ', 'x'):
                 continue
 
+            unit = units[benchmark]
+
             details = "{0:1s} {1:>15s}  {2:>15s} {3:>8s}  ".format(
                 mark,
-                human_value(time_1, "seconds", err=err_1),
-                human_value(time_2, "seconds", err=err_2),
+                human_value(time_1, unit, err=err_1),
+                human_value(time_2, unit, err=err_2),
                 ratio)
 
             if split:


Test patch:
diff --git a/test/example_results/benchmarks.json b/test/example_results/benchmarks.json
index 3c7bfbb..03a5ab3 100644
--- a/test/example_results/benchmarks.json
+++ b/test/example_results/benchmarks.json
@@ -203,7 +203,7 @@
         "param_names": [], 
         "params": [], 
         "timeout": 60.0, 
-        "type": "time", 
+        "type": "track", 
         "unit": "unit"
     },
     "time_AAA_failure": {
@@ -213,7 +213,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_AAA_skip": {
         "code": "foo\n", 
@@ -222,7 +222,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_coordinates.time_latitude": {
         "code": "foo\n", 
@@ -231,7 +231,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_quantity.time_quantity_array_conversion": {
         "code": "foo\n", 
@@ -240,7 +240,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_quantity.time_quantity_init_array": {
         "code": "foo\n", 
@@ -249,7 +249,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_quantity.time_quantity_init_scalar": {
         "code": "foo\n", 
@@ -258,7 +258,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_quantity.time_quantity_scalar_conversion": {
         "code": "foo\n", 
@@ -267,7 +267,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_quantity.time_quantity_ufunc_sin": {
         "code": "foo\n", 
@@ -276,7 +276,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_units.mem_unit": {
         "code": "foo\n", 
@@ -285,7 +285,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_units.time_simple_unit_parse": {
         "code": "foo\n", 
@@ -294,7 +294,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_units.time_unit_compose": {
         "code": "foo\n", 
@@ -303,7 +303,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_units.time_unit_parse": {
         "code": "foo\n", 
@@ -312,7 +312,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_units.time_unit_to": {
         "code": "foo\n", 
@@ -321,7 +321,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_units.time_very_simple_unit_parse": {
         "code": "foo\n", 
@@ -330,7 +330,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_other.time_parameterized": {
         "code": "foo\n", 
@@ -339,7 +339,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_ci_small": {
         "code": "foo\n", 
@@ -348,7 +348,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_ci_big": {
         "code": "foo\n", 
@@ -357,16 +357,16 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit"
+        "unit": "seconds"
     },
     "time_with_version_match": {
         "code": "foo\n", 
-        "name": "time_with_version", 
+        "name": "time_with_version_match", 
         "param_names": [], 
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit",
+        "unit": "seconds",
         "version": "1"
     },
     "time_with_version_mismatch_bench": {
@@ -376,7 +376,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit",
+        "unit": "seconds",
         "version": "2"
     },
     "time_with_version_mismatch_other": {
@@ -386,7 +386,7 @@
         "params": [], 
         "timeout": 60.0, 
         "type": "time", 
-        "unit": "unit",
+        "unit": "seconds",
         "version": "1"
     },
     "version": 2
diff --git a/test/example_results/cheetah/22b920c6-py2.7-Cython-numpy1.8.json b/test/example_results/cheetah/22b920c6-py2.7-Cython-numpy1.8.json
index ab06818..de9ed30 100644
--- a/test/example_results/cheetah/22b920c6-py2.7-Cython-numpy1.8.json
+++ b/test/example_results/cheetah/22b920c6-py2.7-Cython-numpy1.8.json
@@ -37,7 +37,8 @@
 	"time_ci_big": {"result": 1, "stats": [{"ci_99": [0.5, 2.5], "q_25": 0.5, "q_75": 2.5}]},
 	"time_with_version_match": 1,
 	"time_with_version_mismatch_bench": 1,
-	"time_with_version_mismatch_other": 1
+	"time_with_version_mismatch_other": 1,
+	"time_secondary.track_value": 42
     },
     "benchmark_version": {
     	"time_with_version_match": "1",
diff --git a/test/example_results/cheetah/fcf8c079-py2.7-Cython-numpy1.8.json b/test/example_results/cheetah/fcf8c079-py2.7-Cython-numpy1.8.json
index 8319525..2dad502 100644
--- a/test/example_results/cheetah/fcf8c079-py2.7-Cython-numpy1.8.json
+++ b/test/example_results/cheetah/fcf8c079-py2.7-Cython-numpy1.8.json
@@ -34,11 +34,12 @@
 	    "result": [1, 4, null]
 	},
 	"params_examples.ParamSuite.track_value": null,
-	"time_ci_small": {"result": 3, "stats": [{"ci_99": [3.9, 3.1], "q_25": 3, "q_75": 3}]},
+	"time_ci_small": {"result": 3, "stats": [{"ci_99": [3.1, 3.9], "q_25": 3, "q_75": 3}]},
 	"time_ci_big": {"result": 3, "stats": [{"ci_99": [1.5, 3.5], "q_25": 1.5, "q_75": 3.5}]},
 	"time_with_version_match": 3,
 	"time_with_version_mismatch_bench": 3,
-	"time_with_version_mismatch_other": 3
+	"time_with_version_mismatch_other": 3,
+	"time_secondary.track_value": 42
     },
     "benchmark_version": {
     	"time_with_version_match": "1",
diff --git a/test/test_compare.py b/test/test_compare.py
index 0584855..00beca4 100644
--- a/test/test_compare.py
+++ b/test/test_compare.py
@@ -46,6 +46,7 @@ All benchmarks:
            83.6μs           55.4μs     0.66  time_quantity.time_quantity_init_scalar
             282μs            147μs     0.52  time_quantity.time_quantity_scalar_conversion
 +          1.31ms           7.75ms     5.91  time_quantity.time_quantity_ufunc_sin
+               42               42     1.00  time_secondary.track_value
             5.73m            5.73m     1.00  time_units.mem_unit
 +           125μs           3.81ms    30.42  time_units.time_simple_unit_parse
            1.64ms           1.53ms     0.93  time_units.time_unit_compose
@@ -68,12 +69,14 @@ Benchmarks that have stayed the same:
 
        before           after         ratio
      [22b920c6]       [fcf8c079]
+           failed           failed      n/a  time_AAA_failure
               n/a              n/a      n/a  time_AAA_skip
           1.00±1s          3.00±1s    ~3.00  time_ci_big
             1.00s            1.00s     1.00  time_other.time_parameterized(1)
             2.00s            4.00s     2.00  time_other.time_parameterized(2)
            83.6μs           55.4μs     0.66  time_quantity.time_quantity_init_scalar
             282μs            147μs     0.52  time_quantity.time_quantity_scalar_conversion
+               42               42     1.00  time_secondary.track_value
             5.73m            5.73m     1.00  time_units.mem_unit
            1.64ms           1.53ms     0.93  time_units.time_unit_compose
            11.9μs           13.1μs     1.10  time_units.time_very_simple_unit_parse
@@ -83,7 +86,6 @@ Benchmarks that have got worse:
        before           after         ratio
      [22b920c6]       [fcf8c079]
 !             n/a           failed      n/a  params_examples.ParamSuite.track_value
-           failed           failed      n/a  time_AAA_failure
 +         1.00±0s          3.00±0s     3.00  time_ci_small
 !           454μs           failed      n/a  time_coordinates.time_latitude
 !           3.00s           failed      n/a  time_other.time_parameterized(3)
@@ -198,6 +200,9 @@ def test_compare_name_lookup(dvcs_type, capsys, tmpdir):
     for fn in ['feea15ca-py2.7-Cython-numpy1.8.json', 'machine.json']:
         shutil.copyfile(os.path.join(src, fn), os.path.join(dst, fn))
 
+    shutil.copyfile(os.path.join(RESULT_DIR, 'benchmarks.json'),
+                    os.path.join(result_dir, 'benchmarks.json'))
+
     # Copy to different commit
     fn_1 = os.path.join(dst, 'feea15ca-py2.7-Cython-numpy1.8.json')
     fn_2 = os.path.join(dst, commit_hash[:8] + '-py2.7-Cython-numpy1.8.json')
diff --git a/test/test_show.py b/test/test_show.py
index 3eaec04..08e74cc 100644
--- a/test/test_show.py
+++ b/test/test_show.py
@@ -39,7 +39,7 @@ def test_show(capsys, tmpdir):
 
     tools.run_asv_with_conf(conf, 'show', 'fcf8c079')
     text, err = capsys.readouterr()
-    assert "time_ci_small [cheetah/py2.7-numpy1.8]\n  3±0\n\n" in text
+    assert "time_ci_small [cheetah/py2.7-numpy1.8]\n  3.00±0s\n\n" in text
 
     tools.run_asv_with_conf(conf, 'show', 'fcf8c079', '--machine=cheetah',
                             '--bench=time_ci', '--details')
@@ -48,11 +48,11 @@ def test_show(capsys, tmpdir):
     Commit: fcf8c079
 
     time_ci_big [cheetah/py2.7-numpy1.8]
-      3±1
-      ci_99: (1.5, 3.5)
+      3.00±1s
+      ci_99: (1.50s, 3.50s)
 
     time_ci_small [cheetah/py2.7-numpy1.8]
-      3±0
-      ci_99: (3.9, 3.1)
+      3.00±0s
+      ci_99: (3.10s, 3.90s)
     """)
     assert text.strip() == expected.strip()
diff --git a/test/test_workflow.py b/test/test_workflow.py
index d3c35e7..df75b43 100644
--- a/test/test_workflow.py
+++ b/test/test_workflow.py
@@ -189,7 +189,7 @@ def test_continuous(capfd, basic_conf):
 
     text, err = capfd.readouterr()
     assert "SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY" in text
-    assert "+           1.00s            6.00s     6.00  params_examples.track_find_test(2)" in text
+    assert "+               1                6     6.00  params_examples.track_find_test(2)" in text
     assert "params_examples.ClassOne" in text
 
     # Check processes were interleaved (timing benchmark was run twice)