Merge pull request #1937 from pared/1917

metrics: show: do not throw on no metric present on branch

dvc/repo/metrics/show.py

@@ -1,14 +1,19 @@
 from __future__ import unicode_literals
 
+import errno
 import os
 import csv
 import json
 import logging
+
 from jsonpath_ng.ext import parse
 
 from dvc.exceptions import OutputNotFoundError, BadMetricError, NoMetricsError
 from dvc.utils.compat import builtin_str, open, StringIO, csv_reader
 
+NO_METRICS_FILE_AT_REFERENCE_WARNING = (
+    "Metrics file '{}' does not exist at reference: '{}'"
+)
 
 logger = logging.getLogger(__name__)
 
@@ -147,7 +152,7 @@ def _read_metric(fd, typ=None, xpath=None, rel_path=None, branch=None):
         return None
 
 
-def _collect_metrics(self, path, recursive, typ, xpath, branch):
+def _collect_metrics(repo, path, recursive, typ, xpath, branch):
     """Gather all the metric outputs.
 
     Args:
@@ -165,11 +170,11 @@ def _collect_metrics(self, path, recursive, typ, xpath, branch):
             - typ:
             - xpath:
     """
-    outs = [out for stage in self.stages() for out in stage.outs]
+    outs = [out for stage in repo.stages() for out in stage.outs]
 
     if path:
         try:
-            outs = self.find_outs_by_path(path, outs=outs, recursive=recursive)
+            outs = repo.find_outs_by_path(path, outs=outs, recursive=recursive)
         except OutputNotFoundError:
             logger.debug(
                 "stage file not for found for '{}' in branch '{}'".format(
@@ -195,16 +200,7 @@ def _collect_metrics(self, path, recursive, typ, xpath, branch):
     return res
 
 
-def _read_metrics_filesystem(path, typ, xpath, rel_path, branch):
-    if not os.path.exists(path):
-        return None
-    with open(path, "r") as fd:
-        return _read_metric(
-            fd, typ=typ, xpath=xpath, rel_path=rel_path, branch=branch
-        )
-
-
-def _read_metrics(self, metrics, branch):
+def _read_metrics(repo, metrics, branch):
     """Read the content of each metric file and format it.
 
     Args:
@@ -226,15 +222,14 @@ def _read_metrics(self, metrics, branch):
         if not typ:
             typ = os.path.splitext(out.path.lower())[1].replace(".", "")
         if out.use_cache:
-            metric = _read_metrics_filesystem(
-                self.cache.local.get(out.checksum),
-                typ=typ,
-                xpath=xpath,
-                rel_path=out.rel_path,
-                branch=branch,
-            )
+            open_fun = open
+            path = repo.cache.local.get(out.checksum)
         else:
-            with self.tree.open(out.path) as fd:
+            open_fun = repo.tree.open
+            path = out.path
+        try:
+
+            with open_fun(path) as fd:
                 metric = _read_metric(
                     fd,
                     typ=typ,
@@ -242,6 +237,16 @@ def _read_metrics(self, metrics, branch):
                     rel_path=out.rel_path,
                     branch=branch,
                 )
+        except IOError as e:
+            if e.errno == errno.ENOENT:
+                logger.warning(
+                    NO_METRICS_FILE_AT_REFERENCE_WARNING.format(
+                        out.rel_path, branch
+                    )
+                )
+                metric = None
+            else:
+                raise
 
         if not metric:
             continue
@@ -252,7 +257,7 @@ def _read_metrics(self, metrics, branch):
 
 
 def show(
-    self,
+    repo,
     path=None,
     typ=None,
     xpath=None,
@@ -262,9 +267,9 @@ def show(
 ):
     res = {}
 
-    for branch in self.brancher(all_branches=all_branches, all_tags=all_tags):
-        entries = _collect_metrics(self, path, recursive, typ, xpath, branch)
-        metrics = _read_metrics(self, entries, branch)
+    for branch in repo.brancher(all_branches=all_branches, all_tags=all_tags):
+        entries = _collect_metrics(repo, path, recursive, typ, xpath, branch)
+        metrics = _read_metrics(repo, entries, branch)
         if metrics:
             res[branch] = metrics
 

tests/func/test_metrics.py

@@ -6,6 +6,8 @@ import logging
 from dvc.repo import Repo as DvcRepo
 from dvc.main import main
 from dvc.exceptions import DvcException, BadMetricError, NoMetricsError
+from dvc.repo.metrics.show import NO_METRICS_FILE_AT_REFERENCE_WARNING
+from dvc.stage import Stage
 from tests.basic_env import TestDvc
 
 
@@ -775,3 +777,50 @@ class TestMetricsType(TestDvc):
                 self.assertSequenceEqual(ret[branch][file_name], [branch])
             else:
                 self.assertSequenceEqual(ret[branch][file_name], branch)
+
+
+class TestShouldDisplayMetricsEvenIfMetricIsMissing(object):
+    BRANCH_MISSING_METRIC = "missing_metric_branch"
+    METRIC_FILE = "metric"
+    METRIC_FILE_STAGE = METRIC_FILE + Stage.STAGE_FILE_SUFFIX
+
+    def _write_metric(self):
+        with open(self.METRIC_FILE, "w+") as fd:
+            fd.write("0.5")
+            fd.flush()
+
+    def _commit_metric(self, dvc, branch):
+        dvc.scm.add([self.METRIC_FILE_STAGE])
+        dvc.scm.commit("{} commit".format(branch))
+
+    def setUp(self, dvc):
+        dvc.scm.branch(self.BRANCH_MISSING_METRIC)
+
+        self._write_metric()
+
+        ret = main(["run", "-m", self.METRIC_FILE])
+        assert 0 == ret
+
+        self._commit_metric(dvc, "master")
+
+    def test(self, dvc, caplog):
+        self.setUp(dvc)
+
+        dvc.scm.checkout(self.BRANCH_MISSING_METRIC)
+
+        self._write_metric()
+        ret = main(["run", "-M", self.METRIC_FILE])
+        assert 0 == ret
+
+        self._commit_metric(dvc, self.BRANCH_MISSING_METRIC)
+        os.remove(self.METRIC_FILE)
+
+        ret = main(["metrics", "show", "-a"])
+
+        assert (
+            NO_METRICS_FILE_AT_REFERENCE_WARNING.format(
+                self.METRIC_FILE, self.BRANCH_MISSING_METRIC
+            )
+            in caplog.text
+        )
+        assert 0 == ret