metrics: rework metrics implementation (#3658)

Dropping support for all types except json and yaml. No more xpath
support.

Fixes #3572

Author: efiop

dvc/command/metrics.py

@@ -14,11 +14,9 @@
 def show_metrics(
     metrics, all_branches=False, all_tags=False, all_commits=False
 ):
-    """
-    Args:
-        metrics (list): Where each element is either a `list`
-            if an xpath was specified, otherwise a `str`
-    """
+    from flatten_json import flatten
+    from dvc.utils.diff import format_dict
+
     # When `metrics` contains a `None` key, it means that some files
     # specified as `targets` in `repo.metrics.show` didn't contain any metrics.
     missing = metrics.pop(None, None)
@@ -28,21 +26,13 @@ def show_metrics(
             logger.info("{branch}:".format(branch=branch))
 
         for fname, metric in val.items():
-            if isinstance(metric, dict):
-                lines = list(metric.values())
-            elif isinstance(metric, list):
-                lines = metric
-            else:
-                lines = metric.splitlines()
+            if not isinstance(metric, dict):
+                logger.info("\t{}: {}".format(fname, str(metric)))
+                continue
 
-            if len(lines) > 1:
-                logger.info("\t{fname}:".format(fname=fname))
-
-                for line in lines:
-                    logger.info("\t\t{content}".format(content=line))
-
-            else:
-                logger.info("\t{}: {}".format(fname, metric))
+            logger.info("\t{}:".format(fname))
+            for key, value in flatten(format_dict(metric), ".").items():
+                logger.info("\t\t{}: {}".format(key, value))
 
     if missing:
         raise BadMetricError(missing)
@@ -53,35 +43,25 @@ def run(self):
         try:
             metrics = self.repo.metrics.show(
                 self.args.targets,
-                typ=self.args.type,
-                xpath=self.args.xpath,
                 all_branches=self.args.all_branches,
                 all_tags=self.args.all_tags,
                 all_commits=self.args.all_commits,
                 recursive=self.args.recursive,
             )
 
-            show_metrics(
-                metrics,
-                self.args.all_branches,
-                self.args.all_tags,
-                self.args.all_commits,
-            )
-        except DvcException:
-            logger.exception("failed to show metrics")
-            return 1
-
-        return 0
-
+            if self.args.show_json:
+                import json
 
-class CmdMetricsModify(CmdBase):
-    def run(self):
-        try:
-            self.repo.metrics.modify(
-                self.args.path, typ=self.args.type, xpath=self.args.xpath
-            )
+                logger.info(json.dumps(metrics))
+            else:
+                show_metrics(
+                    metrics,
+                    self.args.all_branches,
+                    self.args.all_tags,
+                    self.args.all_commits,
+                )
         except DvcException:
-            logger.exception("failed to modify metric file settings")
+            logger.exception("failed to show metrics")
             return 1
 
         return 0
@@ -90,9 +70,7 @@ def run(self):
 class CmdMetricsAdd(CmdBase):
     def run(self):
         try:
-            self.repo.metrics.add(
-                self.args.path, self.args.type, self.args.xpath
-            )
+            self.repo.metrics.add(self.args.path)
         except DvcException:
             msg = "failed to add metric file '{}'".format(self.args.path)
             logger.exception(msg)
@@ -114,11 +92,14 @@ def run(self):
 
 
 def _show_diff(diff):
+    from collections import OrderedDict
+
     from dvc.utils.diff import table
 
     rows = []
     for fname, mdiff in diff.items():
-        for metric, change in mdiff.items():
+        sorted_mdiff = OrderedDict(sorted(mdiff.items()))
+        for metric, change in sorted_mdiff.items():
             rows.append(
                 [
                     fname,
@@ -138,9 +119,8 @@ def run(self):
                 a_rev=self.args.a_rev,
                 b_rev=self.args.b_rev,
                 targets=self.args.targets,
-                typ=self.args.type,
-                xpath=self.args.xpath,
                 recursive=self.args.recursive,
+                all=self.args.all,
             )
 
             if self.args.show_json:
@@ -185,32 +165,9 @@ def add_parser(subparsers, parent_parser):
         help=METRICS_ADD_HELP,
         formatter_class=argparse.RawDescriptionHelpFormatter,
     )
-    metrics_add_parser.add_argument(
-        "-t", "--type", help="Type of metrics (json/yaml).", metavar="<type>",
-    )
-    metrics_add_parser.add_argument(
-        "-x", "--xpath", help="json/yaml path.", metavar="<path>",
-    )
     metrics_add_parser.add_argument("path", help="Path to a metric file.")
     metrics_add_parser.set_defaults(func=CmdMetricsAdd)
 
-    METRICS_MODIFY_HELP = "Modify metric default formatting."
-    metrics_modify_parser = metrics_subparsers.add_parser(
-        "modify",
-        parents=[parent_parser],
-        description=append_doc_link(METRICS_MODIFY_HELP, "metrics/modify"),
-        help=METRICS_MODIFY_HELP,
-        formatter_class=argparse.RawDescriptionHelpFormatter,
-    )
-    metrics_modify_parser.add_argument(
-        "-t", "--type", help="Type of metrics (json/yaml).", metavar="<type>",
-    )
-    metrics_modify_parser.add_argument(
-        "-x", "--xpath", help="json/yaml path.", metavar="<path>",
-    )
-    metrics_modify_parser.add_argument("path", help="Path to a metric file.")
-    metrics_modify_parser.set_defaults(func=CmdMetricsModify)
-
     METRICS_SHOW_HELP = "Print metrics, with optional formatting."
     metrics_show_parser = metrics_subparsers.add_parser(
         "show",
@@ -224,19 +181,6 @@ def add_parser(subparsers, parent_parser):
         nargs="*",
         help="Metric files or directories (see -R) to show",
     )
-    metrics_show_parser.add_argument(
-        "-t",
-        "--type",
-        help=(
-            "Type of metrics (json/yaml). "
-            "It can be detected by the file extension automatically. "
-            "Unsupported types will be treated as raw."
-        ),
-        metavar="<type>",
-    )
-    metrics_show_parser.add_argument(
-        "-x", "--xpath", help="json/yaml path.", metavar="<path>",
-    )
     metrics_show_parser.add_argument(
         "-a",
         "--all-branches",
@@ -267,6 +211,12 @@ def add_parser(subparsers, parent_parser):
             "metric files."
         ),
     )
+    metrics_show_parser.add_argument(
+        "--show-json",
+        action="store_true",
+        default=False,
+        help="Show output in JSON format.",
+    )
     metrics_show_parser.set_defaults(func=CmdMetricsShow)
 
     METRICS_DIFF_HELP = "Show changes in metrics between commits"
@@ -295,19 +245,6 @@ def add_parser(subparsers, parent_parser):
         ),
         metavar="<paths>",
     )
-    metrics_diff_parser.add_argument(
-        "-t",
-        "--type",
-        help=(
-            "Type of metrics (json/yaml). "
-            "It can be detected by the file extension automatically. "
-            "Unsupported types will be treated as raw."
-        ),
-        metavar="<type>",
-    )
-    metrics_diff_parser.add_argument(
-        "-x", "--xpath", help="json/yaml path.", metavar="<path>",
-    )
     metrics_diff_parser.add_argument(
         "-R",
         "--recursive",
@@ -318,6 +255,12 @@ def add_parser(subparsers, parent_parser):
             "metric files."
         ),
     )
+    metrics_diff_parser.add_argument(
+        "--all",
+        action="store_true",
+        default=False,
+        help="Show unchanged metrics as well.",
+    )
     metrics_diff_parser.add_argument(
         "--show-json",
         action="store_true",

dvc/remote/local.py

@@ -87,6 +87,7 @@ def get(self, md5):
         return self.checksum_to_path_info(md5).url
 
     def exists(self, path_info):
+        assert is_working_tree(self.repo.tree)
         assert path_info.scheme == "local"
         return self.repo.tree.exists(fspath_py35(path_info))
 

dvc/repo/metrics/__init__.py

@@ -7,11 +7,6 @@ def add(self, *args, **kwargs):
 
         return add(self.repo, *args, **kwargs)
 
-    def modify(self, *args, **kwargs):
-        from dvc.repo.metrics.modify import modify
-
-        return modify(self.repo, *args, **kwargs)
-
     def show(self, *args, **kwargs):
         from dvc.repo.metrics.show import show
 

dvc/repo/metrics/add.py

@@ -1,7 +1,5 @@
 from dvc.repo.metrics.modify import modify
 
 
-def add(repo, path, typ=None, xpath=None):
-    if not typ:
-        typ = "raw"
-    modify(repo, path, typ, xpath)
+def add(repo, path):
+    modify(repo, path)

dvc/repo/metrics/diff.py

@@ -1,4 +1,4 @@
-import dvc.utils.diff
+from dvc.utils.diff import format_dict, diff as _diff
 from dvc.exceptions import NoMetricsError
 
 
@@ -13,7 +13,11 @@ def _get_metrics(repo, *args, rev=None, **kwargs):
 
 
 def diff(repo, *args, a_rev=None, b_rev=None, **kwargs):
+    with_unchanged = kwargs.pop("all", False)
+
     old = _get_metrics(repo, *args, **kwargs, rev=(a_rev or "HEAD"))
     new = _get_metrics(repo, *args, **kwargs, rev=b_rev)
 
-    return dvc.utils.diff.diff(old, new)
+    return _diff(
+        format_dict(old), format_dict(new), with_unchanged=with_unchanged
+    )

dvc/repo/metrics/modify.py

@@ -4,8 +4,7 @@
 
 
 @locked
-def modify(repo, path, typ=None, xpath=None, delete=False):
-    supported_types = ["raw", "json", "csv", "tsv", "hcsv", "htsv"]
+def modify(repo, path, delete=False):
     outs = repo.find_outs_by_path(path)
     assert len(outs) == 1
     out = outs[0]
@@ -14,25 +13,6 @@ def modify(repo, path, typ=None, xpath=None, delete=False):
         msg = "output '{}' scheme '{}' is not supported for metrics"
         raise DvcException(msg.format(out.path, out.path_info.scheme))
 
-    if typ is not None:
-        typ = typ.lower().strip()
-        if typ not in ["raw", "json", "csv", "tsv", "hcsv", "htsv"]:
-            msg = (
-                "metric type '{typ}' is not supported, "
-                "must be one of [{types}]"
-            )
-            raise DvcException(
-                msg.format(typ=typ, types=", ".join(supported_types))
-            )
-        if not isinstance(out.metric, dict):
-            out.metric = {}
-        out.metric[out.PARAM_METRIC_TYPE] = typ
-
-    if xpath is not None:
-        if not isinstance(out.metric, dict):
-            out.metric = {}
-        out.metric[out.PARAM_METRIC_XPATH] = xpath
-
     if delete:
         out.metric = None