Merge pull request #912 from efiop/master

metrics: csv support and additional type verification

dvc/cli.py

@@ -477,15 +477,22 @@ def parse_args(argv=None):
                         'path',
                         nargs='?',
                         help='Path to metrics file')
-    metrics_show_parser.add_argument(
+    metrics_show_parser_group = metrics_show_parser.add_mutually_exclusive_group()
+    metrics_show_parser_group.add_argument(
                         '--json-path',
                         help='JSON path')
-    metrics_show_parser.add_argument(
+    metrics_show_parser_group.add_argument(
                         '--tsv-path',
                         help='TSV path \'row,column\'(e.g. \'1,2\')')
-    metrics_show_parser.add_argument(
+    metrics_show_parser_group.add_argument(
                         '--htsv-path',
                         help='Headed TSV path \'row,column\'(e.g. \'Name,3\'')
+    metrics_show_parser_group.add_argument(
+                        '--csv-path',
+                        help='CSV path \'row,column\'(e.g. \'1,2\')')
+    metrics_show_parser_group.add_argument(
+                        '--hcsv-path',
+                        help='Headed CSV path \'row,column\'(e.g. \'Name,3\'')
     metrics_show_parser.add_argument(
                         '--all-branches',
                         action='store_true',

dvc/command/metrics.py

@@ -7,6 +7,8 @@ class CmdMetricsShow(CmdBase):
                                   json_path=self.args.json_path,
                                   tsv_path=self.args.tsv_path,
                                   htsv_path=self.args.htsv_path,
+                                  csv_path=self.args.csv_path,
+                                  hcsv_path=self.args.hcsv_path,
                                   all_branches=self.args.all_branches)
         return 0
 

dvc/output/local.py

@@ -8,6 +8,7 @@ from dvc.dependency.local import DependencyLOCAL
 from dvc.exceptions import DvcException
 from dvc.logger import Logger
 from dvc.utils import remove
+from dvc.istextfile import istextfile
 
 
 class OutputLOCAL(DependencyLOCAL):
@@ -57,9 +58,22 @@ class OutputLOCAL(DependencyLOCAL):
 
         self.project.cache.local.checkout(self.path_info, self.info)
 
+    def _verify_metric(self):
+        if not self.metric:
+            return
+
+        if os.path.isdir(self.path):
+            msg = 'Directory \'{}\' cannot be used as metrics.'
+            raise DvcException(msg.format(self.rel_path))
+
+        if not istextfile(self.path):
+            msg = 'Binary file \'{}\' cannot be used as metrics.'
+            raise DvcException(msg.format(self.rel_path))
+
     def save(self):
         if not self.use_cache:
             super(OutputLOCAL, self).save()
+            self._verify_metric()
             msg = 'Output \'{}\' doesn\'t use cache. Skipping saving.'
             self.project.logger.debug(msg.format(self.rel_path))
             return

dvc/project.py

@@ -400,7 +400,7 @@ class Project(object):
         parser = parse(json_path)
         return [x.value for x in parser.find(json.load(fd))]
 
-    def _do_read_metric_tsv(self, reader, row, col):
+    def _do_read_metric_xsv(self, reader, row, col):
         if col != None and row != None:
             return [reader[row][col]]
         elif col != None:
@@ -409,24 +409,28 @@ class Project(object):
             return reader[row]
         return None
 
-    def _read_metric_htsv(self, fd, htsv_path):
+    def _read_metric_hxsv(self, fd, hxsv_path, delimiter):
         import csv
 
-        col, row = htsv_path.split(',')
+        col, row = hxsv_path.split(',')
         row = int(row)
-        reader = list(csv.DictReader(fd, delimiter='\t'))
-        return self._do_read_metric_tsv(reader, row, col)
+        reader = list(csv.DictReader(fd, delimiter=delimiter))
+        return self._do_read_metric_xsv(reader, row, col)
 
-    def _read_metric_tsv(self, fd, tsv_path):
+    def _read_metric_xsv(self, fd, xsv_path, delimiter):
         import csv
 
-        col, row = tsv_path.split(',')
+        col, row = xsv_path.split(',')
         row = int(row)
         col = int(col)
-        reader = list(csv.reader(fd, delimiter='\t'))
-        return self._do_read_metric_tsv(reader, row, col)
+        reader = list(csv.reader(fd, delimiter=delimiter))
+        return self._do_read_metric_xsv(reader, row, col)
 
-    def _read_metric(self, path, json_path=None, tsv_path=None, htsv_path=None):
+    def _read_metric(self, path, json_path=None,
+                                 tsv_path=None,
+                                 htsv_path=None,
+                                 csv_path=None,
+                                 hcsv_path=None):
         ret = None
 
         if not os.path.exists(path):
@@ -436,10 +440,14 @@ class Project(object):
             with open(path, 'r') as fd:
                 if json_path:
                     ret = self._read_metric_json(fd, json_path)
+                elif csv_path:
+                    ret = self._read_metric_xsv(fd, csv_path, ',')
                 elif tsv_path:
-                    ret = self._read_metric_tsv(fd, tsv_path)
+                    ret = self._read_metric_xsv(fd, tsv_path, '\t')
+                elif hcsv_path:
+                    ret = self._read_metric_hxsv(fd, hcsv_path, ',')
                 elif htsv_path:
-                    ret = self._read_metric_htsv(fd, htsv_path)
+                    ret = self._read_metric_hxsv(fd, htsv_path, '\t')
                 else:
                     ret = fd.read()
         except Exception as exc:
@@ -447,7 +455,12 @@ class Project(object):
 
         return ret
 
-    def metrics_show(self, path=None, json_path=None, tsv_path=None, htsv_path=None, all_branches=False):
+    def metrics_show(self, path=None, json_path=None,
+                                      tsv_path=None,
+                                      htsv_path=None,
+                                      csv_path=None,
+                                      hcsv_path=None,
+                                      all_branches=False):
         res = {}
         for branch in self.scm.brancher(all_branches=all_branches):
             outs = [out for stage in self.active_stages() for out in stage.outs]
@@ -458,7 +471,9 @@ class Project(object):
                 metric = self._read_metric(fname,
                                            json_path=json_path,
                                            tsv_path=tsv_path,
-                                           htsv_path=htsv_path)
+                                           htsv_path=htsv_path,
+                                           csv_path=csv_path,
+                                           hcsv_path=hcsv_path)
                 if not metric:
                     continue
 
@@ -491,6 +506,7 @@ class Project(object):
                     raise DvcException(msg.format(out.rel_path))
 
                 out.metric = val
+                out._verify_metric()
 
             stage.dump()
 

tests/test_metrics.py

@@ -2,6 +2,7 @@ import os
 import json
 
 from dvc.main import main
+from dvc.exceptions import DvcException
 from tests.basic_env import TestDvc
 
 
@@ -19,6 +20,13 @@ class TestMetrics(TestDvc):
             with open('metric_json', 'w+') as fd:
                 json.dump({'branch': branch}, fd)
 
+            with open('metric_csv', 'w+') as fd:
+                fd.write(branch)
+
+            with open('metric_hcsv', 'w+') as fd:
+                fd.write('branch\n')
+                fd.write(branch)
+
             with open('metric_tsv', 'w+') as fd:
                 fd.write(branch)
 
@@ -26,7 +34,7 @@ class TestMetrics(TestDvc):
                 fd.write('branch\n')
                 fd.write(branch)
 
-            self.dvc.scm.add(['metric', 'metric_json', 'metric_tsv', 'metric_htsv'])
+            self.dvc.scm.add(['metric', 'metric_json', 'metric_tsv', 'metric_htsv', 'metric_csv', 'metric_hcsv'])
             self.dvc.scm.commit('metric')
 
         self.dvc.scm.checkout('master')
@@ -56,16 +64,24 @@ class TestMetrics(TestDvc):
         self.assertTrue(ret['bar']['metric_htsv'] == ['bar'])
         self.assertTrue(ret['baz']['metric_htsv'] == ['baz'])
 
+        ret = self.dvc.metrics_show('metric_csv', csv_path='0,0', all_branches=True)
+        self.assertEqual(len(ret), 3)
+        self.assertTrue(ret['foo']['metric_csv'] == ['foo'])
+        self.assertTrue(ret['bar']['metric_csv'] == ['bar'])
+        self.assertTrue(ret['baz']['metric_csv'] == ['baz'])
+
+        ret = self.dvc.metrics_show('metric_hcsv', hcsv_path='branch,0', all_branches=True)
+        self.assertEqual(len(ret), 3)
+        self.assertTrue(ret['foo']['metric_hcsv'] == ['foo'])
+        self.assertTrue(ret['bar']['metric_hcsv'] == ['bar'])
+        self.assertTrue(ret['baz']['metric_hcsv'] == ['baz'])
+
 
 class TestMetricsReproCLI(TestDvc):
     def test(self):
-
-        stage = self.dvc.run(outs_no_cache=['metrics'],
+        stage = self.dvc.run(metrics_no_cache=['metrics'],
                              cmd='python {} {} {}'.format(self.CODE, self.FOO, 'metrics'))
 
-        ret = main(['metrics', 'add', 'metrics'])
-        self.assertEqual(ret, 0)
-
         ret = main(['repro', '-m', stage.path])
         self.assertEqual(ret, 0)
 
@@ -75,6 +91,19 @@ class TestMetricsReproCLI(TestDvc):
         ret = main(['repro', '-f', '-m', stage.path])
         self.assertEqual(ret, 0)
 
+    def test_dir(self):
+        os.mkdir('metrics_dir')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(metrics_no_cache=['metrics_dir'])
+
+    def test_binary(self):
+        with open('metrics_bin', 'w+') as fd:
+            fd.write('\0\0\0\0\0\0\0\0')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(metrics_no_cache=['metrics_bin'])
+
 
 class TestMetricsCLI(TestMetrics):
     def test(self):
@@ -90,3 +119,24 @@ class TestMetricsCLI(TestMetrics):
 
         ret = main(['metrics', 'show', '--all-branches', 'metric_htsv', '--htsv-path', 'branch,0'])
         self.assertEqual(ret, 0)
+
+        ret = main(['metrics', 'show', '--all-branches', 'metric_csv', '--csv-path', '0,0'])
+        self.assertEqual(ret, 0)
+
+        ret = main(['metrics', 'show', '--all-branches', 'metric_hcsv', '--hcsv-path', 'branch,0'])
+        self.assertEqual(ret, 0)
+
+    def test_dir(self):
+        os.mkdir('metrics_dir')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(outs_no_cache=['metrics_dir'])
+            self.dvc.metrics_add('metrics_dir')
+
+    def test_binary(self):
+        with open('metrics_bin', 'w+') as fd:
+            fd.write('\0\0\0\0\0\0\0\0')
+
+        with self.assertRaises(DvcException):
+            self.dvc.run(outs_no_cache=['metrics_bin'])
+            self.dvc.metrics_add('metrics_bin')