metrics: rework metrics implementation (#3658)

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

Fixes #3572

dvc/command/metrics.py

@@ -14,11 +14,9 @@ logger = logging.getLogger(__name__)
 def show_metrics(
     metrics, all_branches=False, all_tags=False, all_commits=False
 ):
-    """
+    from flatten_json import flatten
-    Args:
+    from dvc.utils.diff import format_dict
-        metrics (list): Where each element is either a `list`
+
-            if an xpath was specified, otherwise a `str`
-    """
     # 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):
+            if not isinstance(metric, dict):
-                lines = list(metric.values())
+                logger.info("\t{}: {}".format(fname, str(metric)))
-            elif isinstance(metric, list):
+                continue
-                lines = metric
-            else:
-                lines = metric.splitlines()
 
-            if len(lines) > 1:
+            logger.info("\t{}:".format(fname))
-                logger.info("\t{fname}:".format(fname=fname))
+            for key, value in flatten(format_dict(metric), ".").items():
-
+                logger.info("\t\t{}: {}".format(key, value))
-                for line in lines:
-                    logger.info("\t\t{content}".format(content=line))
-
-            else:
-                logger.info("\t{}: {}".format(fname, metric))
 
     if missing:
         raise BadMetricError(missing)
@@ -53,20 +43,23 @@ class CmdMetricsShow(CmdBase):
         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(
+            if self.args.show_json:
-                metrics,
+                import json
-                self.args.all_branches,
+
-                self.args.all_tags,
+                logger.info(json.dumps(metrics))
-                self.args.all_commits,
+            else:
-            )
+                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
@@ -74,25 +67,10 @@ class CmdMetricsShow(CmdBase):
         return 0
 
 
-class CmdMetricsModify(CmdBase):
-    def run(self):
-        try:
-            self.repo.metrics.modify(
-                self.args.path, typ=self.args.type, xpath=self.args.xpath
-            )
-        except DvcException:
-            logger.exception("failed to modify metric file settings")
-            return 1
-
-        return 0
-
-
 class CmdMetricsAdd(CmdBase):
     def run(self):
         try:
-            self.repo.metrics.add(
+            self.repo.metrics.add(self.args.path)
-                self.args.path, self.args.type, self.args.xpath
-            )
         except DvcException:
             msg = "failed to add metric file '{}'".format(self.args.path)
             logger.exception(msg)
@@ -114,11 +92,14 @@ class CmdMetricsRemove(CmdBase):
 
 
 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 @@ class CmdMetricsDiff(CmdBase):
                 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 @@ class RemoteLOCAL(RemoteBASE):
         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 @@ class Metrics(object):
 
         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):
+def add(repo, path):
-    if not typ:
+    modify(repo, path)
-        typ = "raw"
-    modify(repo, path, typ, xpath)

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 @@ from dvc.repo import locked
 
 
 @locked
-def modify(repo, path, typ=None, xpath=None, delete=False):
+def modify(repo, path, delete=False):
-    supported_types = ["raw", "json", "csv", "tsv", "hcsv", "htsv"]
     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
 

dvc/repo/metrics/show.py

@@ -1,258 +1,79 @@
-import csv
+import yaml
-import errno
-import json
 import logging
 import os
-import io
-
-from jsonpath_ng.ext import parse
 
+from dvc.path_info import PathInfo
+from dvc.compat import fspath_py35
 from dvc.exceptions import NoMetricsError
-from dvc.exceptions import OutputNotFoundError
 from dvc.repo import locked
+from dvc.repo.tree import RepoTree
 
-NO_METRICS_FILE_AT_REFERENCE_WARNING = (
-    "Metrics file '{}' does not exist at the reference '{}'."
-)
 
 logger = logging.getLogger(__name__)
 
 
-def _read_metric_json(fd, json_path):
+def _collect_metrics(repo, targets, recursive):
-    parser = parse(json_path)
+    metrics = set()
-    return {str(x.full_path): x.value for x in parser.find(json.load(fd))}
+
+    for stage in repo.stages:
+        for out in stage.outs:
+            if not out.metric:
+                continue
+
+            metrics.add(out.path_info)
+
+    if not targets:
+        return list(metrics)
+
+    target_infos = [PathInfo(os.path.abspath(target)) for target in targets]
+
+    def _filter(path_info):
+        for info in target_infos:
+            func = path_info.isin_or_eq if recursive else path_info.__eq__
+            if func(info):
+                return True
+        return False
+
+    return list(filter(_filter, metrics))
 
 
-def _get_values(row):
+def _extract_metrics(metrics):
-    if isinstance(row, dict):
+    if isinstance(metrics, (int, float)):
-        return list(row.values())
+        return metrics
-    else:
-        return row
 
+    if not isinstance(metrics, dict):
+        return None
 
-def _do_read_metric_xsv(reader, row, col):
+    ret = {}
-    if col is not None and row is not None:
+    for key, val in metrics.items():
-        return [reader[row][col]]
+        m = _extract_metrics(val)
-    elif col is not None:
+        if m:
-        return [r[col] for r in reader]
+            ret[key] = m
-    elif row is not None:
-        return _get_values(reader[row])
-    return [_get_values(r) for r in reader]
 
-
-def _read_metric_hxsv(fd, hxsv_path, delimiter):
-    indices = hxsv_path.split(",")
-    row = indices[0]
-    row = int(row) if row else None
-    col = indices[1] if len(indices) > 1 and indices[1] else None
-    reader = list(csv.DictReader(fd, delimiter=delimiter))
-    return _do_read_metric_xsv(reader, row, col)
-
-
-def _read_metric_xsv(fd, xsv_path, delimiter):
-    indices = xsv_path.split(",")
-    row = indices[0]
-    row = int(row) if row else None
-    col = int(indices[1]) if len(indices) > 1 and indices[1] else None
-    reader = list(csv.reader(fd, delimiter=delimiter))
-    return _do_read_metric_xsv(reader, row, col)
-
-
-def _read_typed_metric(typ, xpath, fd):
-    if typ == "json":
-        ret = _read_metric_json(fd, xpath)
-    elif typ == "csv":
-        ret = _read_metric_xsv(fd, xpath, ",")
-    elif typ == "tsv":
-        ret = _read_metric_xsv(fd, xpath, "\t")
-    elif typ == "hcsv":
-        ret = _read_metric_hxsv(fd, xpath, ",")
-    elif typ == "htsv":
-        ret = _read_metric_hxsv(fd, xpath, "\t")
-    else:
-        ret = fd.read().strip()
     return ret
 
 
-def _format_csv(content, delimiter):
+def _read_metrics(repo, metrics, rev):
-    """Format delimited text to have same column width.
+    tree = RepoTree(repo)
 
-    Args:
-        content (str): The content of a metric.
-        delimiter (str): Value separator
-
-    Returns:
-        str: Formatted content.
-
-    Example:
-
-        >>> content = (
-            "value_mse,deviation_mse,data_set\n"
-            "0.421601,0.173461,train\n"
-            "0.67528,0.289545,testing\n"
-            "0.671502,0.297848,validation\n"
-        )
-        >>> _format_csv(content, ",")
-
-        "value_mse  deviation_mse   data_set\n"
-        "0.421601   0.173461        train\n"
-        "0.67528    0.289545        testing\n"
-        "0.671502   0.297848        validation\n"
-    """
-    reader = csv.reader(io.StringIO(content), delimiter=delimiter)
-    rows = [row for row in reader]
-    max_widths = [max(map(len, column)) for column in zip(*rows)]
-
-    lines = [
-        " ".join(
-            "{entry:{width}}".format(entry=entry, width=width + 2)
-            for entry, width in zip(row, max_widths)
-        )
-        for row in rows
-    ]
-
-    return "\n".join(lines)
-
-
-def _format_output(content, typ):
-    """Tabularize the content according to its type.
-
-    Args:
-        content (str): The content of a metric.
-        typ (str): The type of metric -- (raw|json|tsv|htsv|csv|hcsv).
-
-    Returns:
-        str: Content in a raw or tabular format.
-    """
-
-    if "csv" in typ:
-        return _format_csv(content, delimiter=",")
-
-    if "tsv" in typ:
-        return _format_csv(content, delimiter="\t")
-
-    return content
-
-
-def _read_metric(fd, typ=None, xpath=None, fname=None, branch=None):
-    typ = typ.lower().strip() if typ else typ
-    try:
-        if xpath:
-            return _read_typed_metric(typ, xpath.strip(), fd)
-        else:
-            return _format_output(fd.read().strip(), typ)
-    # Json path library has to be replaced or wrapped in
-    # order to fix this too broad except clause.
-    except Exception:
-        logger.exception(
-            "unable to read metric in '{}' in branch '{}'".format(
-                fname, branch
-            )
-        )
-        return None
-
-
-def _collect_metrics(repo, path, recursive, typ, xpath, branch):
-    """Gather all the metric outputs.
-
-    Args:
-        path (str): Path to a metric file or a directory.
-        recursive (bool): If path is a directory, do a recursive search for
-            metrics on the given path.
-        typ (str): The type that will be used to interpret the metric file,
-            one of the followings - (raw|json|tsv|htsv|csv|hcsv).
-        xpath (str): Path to search for.
-        branch (str): Branch to look up for metrics.
-
-    Returns:
-        list(tuple): (output, typ, xpath)
-            - output:
-            - typ:
-            - xpath:
-    """
-    outs = [out for stage in repo.stages for out in stage.outs]
-
-    if path:
-        try:
-            outs = repo.find_outs_by_path(path, outs=outs, recursive=recursive)
-        except OutputNotFoundError:
-            logger.debug(
-                "DVC-file not for found for '{}' in branch '{}'".format(
-                    path, branch
-                )
-            )
-            return []
-
-    res = []
-    for o in outs:
-        if not o.metric:
-            continue
-
-        # NOTE this case assumes that typ has not been provided in CLI call
-        # and one of the following cases:
-        # - stage file contains metric type
-        # - typ will be read from file extension later
-        if not typ and isinstance(o.metric, dict):
-            t = o.metric.get(o.PARAM_METRIC_TYPE, typ)
-            # NOTE user might want to check different xpath, hence xpath first
-            x = xpath or o.metric.get(o.PARAM_METRIC_XPATH)
-        else:
-            t = typ
-            x = xpath
-
-        res.append((o, t, x))
-
-    return res
-
-
-def _read_metrics(repo, metrics, branch):
-    """Read the content of each metric file and format it.
-
-    Args:
-        metrics (list): List of metric touples
-        branch (str): Branch to look up for metrics.
-
-    Returns:
-        A dict mapping keys with metrics path name and content.
-        For example:
-
-        {'metric.csv': ("value_mse  deviation_mse   data_set\n"
-                        "0.421601   0.173461        train\n"
-                        "0.67528    0.289545        testing\n"
-                        "0.671502   0.297848        validation\n")}
-    """
     res = {}
-    for out, typ, xpath in metrics:
+    for metric in metrics:
-        assert out.scheme == "local"
+        if not tree.exists(fspath_py35(metric)):
-        if not typ:
-            typ = os.path.splitext(out.fspath.lower())[1].replace(".", "")
-        if out.use_cache:
-            open_fun = open
-            path = repo.cache.local.get(out.checksum)
-        else:
-            open_fun = repo.tree.open
-            path = out.fspath
-        try:
-
-            with open_fun(path) as fd:
-                metric = _read_metric(
-                    fd, typ=typ, xpath=xpath, fname=str(out), branch=branch
-                )
-        except IOError as e:
-            if e.errno == errno.ENOENT:
-                logger.warning(
-                    NO_METRICS_FILE_AT_REFERENCE_WARNING.format(
-                        out.path_info, branch
-                    )
-                )
-                metric = None
-            else:
-                raise
-
-        if not metric:
             continue
 
-        res[str(out)] = metric
+        with tree.open(fspath_py35(metric), "r") as fobj:
+            try:
+                # NOTE this also supports JSON
+                val = yaml.safe_load(fobj)
+            except yaml.YAMLError:
+                logger.debug(
+                    "failed to read '%s' on '%s'", metric, rev, exc_info=True
+                )
+                continue
+
+            val = _extract_metrics(val)
+            if val:
+                res[str(metric)] = val
 
     return res
 
@@ -261,8 +82,6 @@ def _read_metrics(repo, metrics, branch):
 def show(
     repo,
     targets=None,
-    typ=None,
-    xpath=None,
     all_branches=False,
     all_tags=False,
     recursive=False,
@@ -270,34 +89,20 @@ def show(
     all_commits=False,
 ):
     res = {}
-    found = set()
 
-    if not targets:
+    for rev in repo.brancher(
-        # Iterate once to call `_collect_metrics` on all the stages
-        targets = [None]
-
-    for branch in repo.brancher(
         revs=revs,
         all_branches=all_branches,
         all_tags=all_tags,
         all_commits=all_commits,
     ):
-        metrics = {}
+        metrics = _collect_metrics(repo, targets, recursive)
+        vals = _read_metrics(repo, metrics, rev)
 
-        for target in targets:
+        if vals:
-            entries = _collect_metrics(
+            res[rev] = vals
-                repo, target, recursive, typ, xpath, branch
-            )
-            metric = _read_metrics(repo, entries, branch)
 
-            if metric:
+    if not res:
-                found.add(target)
-                metrics.update(metric)
-
-        if metrics:
-            res[branch] = metrics
-
-    if not res and not any(targets):
         raise NoMetricsError()
 
     # Hide working tree metrics if they are the same as in the active branch
@@ -309,9 +114,4 @@ def show(
         if res.get("working tree") == res.get(active_branch):
             res.pop("working tree", None)
 
-    missing = set(targets) - found
-
-    if missing:
-        res[None] = missing
-
     return res

dvc/repo/params/diff.py

@@ -1,4 +1,4 @@
-import dvc.utils.diff
+from dvc.utils.diff import format_dict, diff as _diff
 from .show import NoParamsError
 
 
@@ -10,25 +10,12 @@ def _get_params(repo, *args, rev=None, **kwargs):
         return {}
 
 
-def _format(params):
-    ret = {}
-    for key, val in params.items():
-        if isinstance(val, dict):
-            new_val = _format(val)
-        elif isinstance(val, list):
-            new_val = str(val)
-        else:
-            new_val = val
-        ret[key] = new_val
-    return ret
-
-
 def diff(repo, *args, a_rev=None, b_rev=None, **kwargs):
     with_unchanged = kwargs.pop("all", False)
 
     old = _get_params(repo, *args, **kwargs, rev=(a_rev or "HEAD"))
     new = _get_params(repo, *args, **kwargs, rev=b_rev)
 
-    return dvc.utils.diff.diff(
+    return _diff(
-        _format(old), _format(new), with_unchanged=with_unchanged
+        format_dict(old), format_dict(new), with_unchanged=with_unchanged
     )

dvc/repo/tree.py

@@ -1,7 +1,7 @@
 import os
 import errno
 
-from dvc.scm.tree import BaseTree
+from dvc.scm.tree import BaseTree, WorkingTree
 from dvc.path_info import PathInfo
 from dvc.exceptions import OutputNotFoundError
 
@@ -32,10 +32,17 @@ class DvcTree(BaseTree):
             raise IOError(errno.EISDIR)
 
         out = outs[0]
-        if not out.changed_cache():
+        # temporary hack to make cache use WorkingTree and not GitTree, because
-            return open(out.cache_path, mode=mode, encoding=encoding)
+        # cache dir doesn't exist in the latter.
+        saved_tree = self.repo.tree
+        self.repo.tree = WorkingTree(self.repo.root_dir)
+        try:
+            if out.changed_cache():
+                raise FileNotFoundError
+        finally:
+            self.repo.tree = saved_tree
 
-        raise FileNotFoundError
+        return open(out.cache_path, mode=mode, encoding=encoding)
 
     def exists(self, path):
         try:
@@ -115,3 +122,20 @@ class DvcTree(BaseTree):
 
     def isexec(self, path):
         return False
+
+
+class RepoTree(BaseTree):
+    def __init__(self, repo):
+        self.repo = repo
+        self.dvctree = DvcTree(repo)
+
+    def open(self, *args, **kwargs):
+        try:
+            return self.dvctree.open(*args, **kwargs)
+        except FileNotFoundError:
+            pass
+
+        return self.repo.tree.open(*args, **kwargs)
+
+    def exists(self, path):
+        return self.repo.tree.exists(path) or self.dvctree.exists(path)

dvc/utils/diff.py

@@ -1,8 +1,6 @@
 import json
 from collections import defaultdict
 
-from flatten_json import flatten
-
 
 def _parse(raw):
     if raw is None or isinstance(raw, (dict, list, int, float)):
@@ -37,7 +35,9 @@ def _flatten(d):
         return defaultdict(lambda: None)
 
     if isinstance(d, dict):
-        return defaultdict(lambda: None, flatten(d, "."))
+        from flatten_json import flatten as fltn
+
+        return defaultdict(lambda: None, fltn(d, "."))
 
     return defaultdict(lambda: "unable to parse")
 
@@ -103,3 +103,16 @@ def table(header, rows):
     t.add_rows([header] + rows)
 
     return t.draw()
+
+
+def format_dict(d):
+    ret = {}
+    for key, val in d.items():
+        if isinstance(val, dict):
+            new_val = format_dict(val)
+        elif isinstance(val, list):
+            new_val = str(val)
+        else:
+            new_val = val
+        ret[key] = new_val
+    return ret

tests/func/metrics/test_diff.py

@@ -0,0 +1,135 @@
+import json
+import yaml
+
+
+def test_metrics_diff_simple(tmp_dir, scm, dvc):
+    def _gen(val):
+        tmp_dir.gen({"m.yaml": str(val)})
+        dvc.run(cmd="", metrics=["m.yaml"])
+        dvc.scm.add(["m.yaml.dvc"])
+        dvc.scm.commit(str(val))
+
+    _gen(1)
+    _gen(2)
+    _gen(3)
+
+    expected = {"m.yaml": {"": {"old": 1, "new": 3, "diff": 2}}}
+
+    assert dvc.metrics.diff(a_rev="HEAD~2") == expected
+
+
+def test_metrics_diff_yaml(tmp_dir, scm, dvc):
+    def _gen(val):
+        metrics = {"a": {"b": {"c": val, "d": 1, "e": str(val)}}}
+        tmp_dir.gen({"m.yaml": yaml.dump(metrics)})
+        dvc.run(cmd="", metrics=["m.yaml"])
+        dvc.scm.add(["m.yaml.dvc"])
+        dvc.scm.commit(str(val))
+
+    _gen(1)
+    _gen(2)
+    _gen(3)
+
+    expected = {"m.yaml": {"a.b.c": {"old": 1, "new": 3, "diff": 2}}}
+
+    assert dvc.metrics.diff(a_rev="HEAD~2") == expected
+
+
+def test_metrics_diff_json(tmp_dir, scm, dvc):
+    def _gen(val):
+        metrics = {"a": {"b": {"c": val, "d": 1, "e": str(val)}}}
+        tmp_dir.gen({"m.json": json.dumps(metrics)})
+        dvc.run(cmd="", metrics=["m.json"])
+        dvc.scm.add(["m.json.dvc"])
+        dvc.scm.commit(str(val))
+
+    _gen(1)
+    _gen(2)
+    _gen(3)
+
+    expected = {"m.json": {"a.b.c": {"old": 1, "new": 3, "diff": 2}}}
+    assert dvc.metrics.diff(a_rev="HEAD~2") == expected
+
+
+def test_metrics_diff_json_unchanged(tmp_dir, scm, dvc):
+    def _gen(val):
+        metrics = {"a": {"b": {"c": val, "d": 1, "e": str(val)}}}
+        tmp_dir.gen({"m.json": json.dumps(metrics)})
+        dvc.run(cmd="", metrics=["m.json"])
+        dvc.scm.add(["m.json.dvc"])
+        dvc.scm.commit(str(val))
+
+    _gen(1)
+    _gen(2)
+    _gen(1)
+
+    assert dvc.metrics.diff(a_rev="HEAD~2") == {}
+
+
+def test_metrics_diff_broken_json(tmp_dir, scm, dvc):
+    metrics = {"a": {"b": {"c": 1, "d": 1, "e": "3"}}}
+    tmp_dir.gen({"m.json": json.dumps(metrics)})
+    dvc.run(cmd="", metrics_no_cache=["m.json"])
+    dvc.scm.add(["m.json.dvc", "m.json"])
+    dvc.scm.commit("add metrics")
+
+    (tmp_dir / "m.json").write_text(json.dumps(metrics) + "ma\nlformed\n")
+
+    assert dvc.metrics.diff() == {
+        "m.json": {
+            "a.b.c": {"old": 1, "new": None},
+            "a.b.d": {"old": 1, "new": None},
+        }
+    }
+
+
+def test_metrics_diff_no_metrics(tmp_dir, scm, dvc):
+    tmp_dir.scm_gen({"foo": "foo"}, commit="add foo")
+    assert dvc.metrics.diff(a_rev="HEAD~1") == {}
+
+
+def test_metrics_diff_new_metric(tmp_dir, scm, dvc):
+    metrics = {"a": {"b": {"c": 1, "d": 1, "e": "3"}}}
+    tmp_dir.gen({"m.json": json.dumps(metrics)})
+    dvc.run(cmd="", metrics_no_cache=["m.json"])
+
+    assert dvc.metrics.diff() == {
+        "m.json": {
+            "a.b.c": {"old": None, "new": 1},
+            "a.b.d": {"old": None, "new": 1},
+        }
+    }
+
+
+def test_metrics_diff_deleted_metric(tmp_dir, scm, dvc):
+    metrics = {"a": {"b": {"c": 1, "d": 1, "e": "3"}}}
+    tmp_dir.gen({"m.json": json.dumps(metrics)})
+    dvc.run(cmd="", metrics_no_cache=["m.json"])
+    dvc.scm.add(["m.json.dvc", "m.json"])
+    dvc.scm.commit("add metrics")
+
+    (tmp_dir / "m.json").unlink()
+
+    assert dvc.metrics.diff() == {
+        "m.json": {
+            "a.b.c": {"old": 1, "new": None},
+            "a.b.d": {"old": 1, "new": None},
+        }
+    }
+
+
+def test_metrics_diff_with_unchanged(tmp_dir, scm, dvc):
+    tmp_dir.gen("metrics.yaml", "foo: 1\nxyz: 10")
+    dvc.run(metrics_no_cache=["metrics.yaml"])
+    scm.add(["metrics.yaml", "metrics.yaml.dvc"])
+    scm.commit("1")
+
+    tmp_dir.scm_gen("metrics.yaml", "foo: 2\nxyz: 10", commit="2")
+    tmp_dir.scm_gen("metrics.yaml", "foo: 3\nxyz: 10", commit="3")
+
+    assert dvc.metrics.diff(a_rev="HEAD~2", all=True) == {
+        "metrics.yaml": {
+            "foo": {"old": 1, "new": 3, "diff": 2},
+            "xyz": {"old": 10, "new": 10, "diff": 0},
+        }
+    }

tests/func/metrics/test_show.py

@@ -0,0 +1,50 @@
+import pytest
+
+from dvc.repo.metrics.show import NoMetricsError
+
+
+def test_show_empty(dvc):
+    with pytest.raises(NoMetricsError):
+        dvc.metrics.show()
+
+
+def test_show_simple(tmp_dir, dvc):
+    tmp_dir.gen("metrics.yaml", "1.1")
+    dvc.run(metrics=["metrics.yaml"])
+    assert dvc.metrics.show() == {"": {"metrics.yaml": 1.1}}
+
+
+def test_show(tmp_dir, dvc):
+    tmp_dir.gen("metrics.yaml", "foo: 1.1")
+    dvc.run(metrics=["metrics.yaml"])
+    assert dvc.metrics.show() == {"": {"metrics.yaml": {"foo": 1.1}}}
+
+
+def test_show_multiple(tmp_dir, dvc):
+    tmp_dir.gen("foo", "foo: 1\n")
+    tmp_dir.gen("baz", "baz: 2\n")
+    dvc.run(fname="foo.dvc", metrics=["foo"])
+    dvc.run(fname="baz.dvc", metrics=["baz"])
+    assert dvc.metrics.show() == {"": {"foo": {"foo": 1}, "baz": {"baz": 2}}}
+
+
+def test_show_invalid_metric(tmp_dir, dvc):
+    tmp_dir.gen("metrics.yaml", "foo:\n- bar\n- baz\nxyz: string")
+    dvc.run(metrics=["metrics.yaml"])
+    with pytest.raises(NoMetricsError):
+        dvc.metrics.show()
+
+
+def test_show_branch(tmp_dir, scm, dvc):
+    tmp_dir.gen("metrics.yaml", "foo: 1")
+    dvc.run(metrics_no_cache=["metrics.yaml"])
+    scm.add(["metrics.yaml", "metrics.yaml.dvc"])
+    scm.commit("init")
+
+    with tmp_dir.branch("branch", new=True):
+        tmp_dir.scm_gen("metrics.yaml", "foo: 2", commit="branch")
+
+    assert dvc.metrics.show(revs=["branch"]) == {
+        "working tree": {"metrics.yaml": {"foo": 1}},
+        "branch": {"metrics.yaml": {"foo": 2}},
+    }

tests/func/test_init.py

@@ -69,10 +69,11 @@ def test_init_no_scm_cli(tmp_dir):
 
 
 def test_init_quiet_should_not_display_welcome_screen(tmp_dir, scm, caplog):
-    ret = main(["init", "--quiet"])
+    with caplog.at_level(logging.INFO, logger="dvc"):
+        ret = main(["init", "--quiet"])
 
-    assert 0 == ret
+        assert 0 == ret
-    assert "" == caplog.text
+        assert "" == caplog.text
 
 
 def test_allow_init_dvc_subdir(tmp_dir, scm, monkeypatch):

tests/unit/command/test_metrics.py

@@ -9,11 +9,8 @@ def test_metrics_diff(dvc, mocker):
             "diff",
             "HEAD~10",
             "HEAD~1",
-            "-t",
-            "json",
-            "-x",
-            "x.path",
             "-R",
+            "--all",
             "--show-json",
             "--targets",
             "target1",
@@ -32,9 +29,8 @@ def test_metrics_diff(dvc, mocker):
         targets=["target1", "target2"],
         a_rev="HEAD~10",
         b_rev="HEAD~1",
-        typ="json",
-        xpath="x.path",
         recursive=True,
+        all=True,
     )
 
 
@@ -90,10 +86,6 @@ def test_metrics_show(dvc, mocker):
         [
             "metrics",
             "show",
-            "-t",
-            "json",
-            "-x",
-            "x.path",
             "-R",
             "--all-tags",
             "--all-branches",
@@ -112,8 +104,6 @@ def test_metrics_show(dvc, mocker):
     m.assert_called_once_with(
         cmd.repo,
         ["target1", "target2"],
-        typ="json",
-        xpath="x.path",
         recursive=True,
         all_tags=True,
         all_branches=True,
@@ -128,3 +118,20 @@ def test_metrics_diff_prec():
         "   Path      Metric   Value    Change\n"
         "other.json   a.b      0.0043   0.0001"
     )
+
+
+def test_metrics_diff_sorted():
+    assert _show_diff(
+        {
+            "metrics.yaml": {
+                "x.b": {"old": 5, "new": 6, "diff": 1},
+                "a.d.e": {"old": 3, "new": 4, "diff": 1},
+                "a.b.c": {"old": 1, "new": 2, "diff": 1},
+            }
+        }
+    ) == (
+        "    Path       Metric   Value   Change\n"
+        "metrics.yaml   a.b.c    2       1     \n"
+        "metrics.yaml   a.d.e    4       1     \n"
+        "metrics.yaml   x.b      6       1     "
+    )

tests/unit/repo/test_tree.py

@@ -24,6 +24,26 @@ def test_open(tmp_dir, dvc):
         assert fobj.read() == "foo"
 
 
+def test_open_in_history(tmp_dir, scm, dvc):
+    tmp_dir.gen("foo", "foo")
+    dvc.add("foo")
+    dvc.scm.add(["foo.dvc", ".gitignore"])
+    dvc.scm.commit("foo")
+
+    tmp_dir.gen("foo", "foofoo")
+    dvc.add("foo")
+    dvc.scm.add(["foo.dvc", ".gitignore"])
+    dvc.scm.commit("foofoo")
+
+    for rev in dvc.brancher(revs=["HEAD~1"]):
+        if rev == "working tree":
+            continue
+
+        tree = DvcTree(dvc)
+        with tree.open("foo", "r") as fobj:
+            assert fobj.read() == "foo"
+
+
 def test_isdir_isfile(tmp_dir, dvc):
     tmp_dir.gen({"datafile": "data", "datadir": {"foo": "foo", "bar": "bar"}})