Repository: iterative/dvc
Pull request #1605 (2019-02-12 02:35:14)

Fix metrics show all braches/tags behavior when target is not specified
* Fixes https://github.com/iterative/dvc/issues/1590#issuecomment-462323771
* Refactoring - simplifies metrics show code removes backward compatible behavior - all files that metrics show takes **must** be metrics files in DVC sense - there **must**  a DVC files that specifies them as a metric file.


Patch:
diff --git a/dvc/project.py b/dvc/project.py
index a81309e7f..f808a31e3 100644
--- a/dvc/project.py
+++ b/dvc/project.py
@@ -1127,7 +1127,6 @@ class Project(object):
         for branch in self.scm.brancher(
             all_branches=all_branches, all_tags=all_tags
         ):
-
             astages = self.active_stages()
             outs = [out for stage in astages for out in stage.outs]
 
@@ -1135,44 +1134,28 @@ class Project(object):
                 outs = self._find_output_by_path(
                     path, outs=outs, recursive=recursive
                 )
-                stages = [out.stage.path for out in outs]
-                entries = []
-                for out in outs:
-                    if all(
-                        [out.metric, not typ, isinstance(out.metric, dict)]
-                    ):
-                        entries += [
-                            (
-                                out.path,
-                                out.metric.get(out.PARAM_METRIC_TYPE, None),
-                                out.metric.get(out.PARAM_METRIC_XPATH, None),
-                            )
-                        ]
-                    else:
-                        entries += [(out.path, typ, xpath)]
-                if not entries:
-                    if os.path.isdir(path):
-                        logger.warning(
-                            "Path '{path}' is a directory. "
-                            "Consider running with '-R'.".format(path=path)
-                        )
-                        return {}
 
-                    else:
-                        entries += [(path, typ, xpath)]
+            metrics = filter(lambda o: o.metric, outs)
+            stages = set()
+            entries = []
 
-            else:
-                metrics = filter(lambda o: o.metric, outs)
-                stages = None
-                entries = []
-                for o in metrics:
-                    if not typ and isinstance(o.metric, dict):
-                        t = o.metric.get(o.PARAM_METRIC_TYPE, typ)
-                        x = o.metric.get(o.PARAM_METRIC_XPATH, xpath)
-                    else:
-                        t = typ
-                        x = xpath
-                    entries.append((o.path, t, x))
+            for o in metrics:
+                if not typ and isinstance(o.metric, dict):
+                    t = o.metric.get(o.PARAM_METRIC_TYPE, typ)
+                    x = o.metric.get(o.PARAM_METRIC_XPATH, xpath)
+                else:
+                    t = typ
+                    x = xpath
+                entries.append((o.path, t, x))
+                stages.add(o.stage.path)
+
+            if path and not entries:
+                if os.path.isdir(path):
+                    logger.warning(
+                        "Path '{path}' is a directory. "
+                        "Consider running with '-R'.".format(path=path)
+                    )
+                    return {}
 
             for fname, t, x in entries:
                 if stages:
@@ -1202,7 +1185,9 @@ class Project(object):
             return res
 
         if path:
-            msg = "file '{}' does not exist or malformed".format(path)
+            msg = "file '{}' does not exist, not a metric file or is malformed".format(
+                path
+            )
         else:
             msg = (
                 "no metric files in this repository."


Test patch:
diff --git a/tests/test_metrics.py b/tests/test_metrics.py
index 77ff4e355..577db3337 100644
--- a/tests/test_metrics.py
+++ b/tests/test_metrics.py
@@ -35,16 +35,19 @@ class TestMetrics(TestDvc):
                 fd.write("branch\n")
                 fd.write(branch)
 
-            self.dvc.scm.add(
-                [
-                    "metric",
-                    "metric_json",
-                    "metric_tsv",
-                    "metric_htsv",
-                    "metric_csv",
-                    "metric_hcsv",
-                ]
-            )
+            files = [
+                "metric",
+                "metric_json",
+                "metric_tsv",
+                "metric_htsv",
+                "metric_csv",
+                "metric_hcsv",
+            ]
+
+            self.dvc.run(metrics_no_cache=files, overwrite=True)
+
+            self.dvc.scm.add(files + ["metric.dvc"])
+
             self.dvc.scm.commit("metric")
 
         self.dvc.scm.checkout("master")
@@ -367,6 +370,7 @@ class TestCachedMetrics(TestDvc):
             json.dump({"metrics": branch}, fd)
 
         stages = self.dvc.add("metrics.json")
+        self.dvc.metrics_add("metrics.json", typ="json", xpath="metrics")
         self.assertEqual(len(stages), 1)
         stage = stages[0]
         self.assertIsNotNone(stage)
@@ -424,6 +428,19 @@ class TestCachedMetrics(TestDvc):
             },
         )
 
+        res = self.dvc.metrics_show(
+            "", all_branches=True, typ="json", xpath="metrics"
+        )
+
+        self.assertEqual(
+            res,
+            {
+                "master": {"metrics.json": ["master"]},
+                "one": {"metrics.json": ["one"]},
+                "two": {"metrics.json": ["two"]},
+            },
+        )
+
     def test_add(self):
         self._test_metrics(self._do_add)