From 0e46edd9bb46d0464045876fd4df01cb311d4cad Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Fri, 9 Aug 2019 21:54:28 -0500 Subject: [PATCH] fail silently but with a more sound error message * Add a `_missing` key to the return value of `repo.metrics.show` to include targets not recognized as metrics or not found. --- dvc/command/metrics.py | 28 ++++++------------- dvc/exceptions.py | 10 +++++++ dvc/repo/metrics/show.py | 56 ++++++++++++++++++++++---------------- tests/func/test_metrics.py | 2 +- 4 files changed, 52 insertions(+), 44 deletions(-) diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index 73bd9ae9f..cfd1857ad 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -2,9 +2,8 @@ from __future__ import unicode_literals import argparse import logging -import itertools -from dvc.exceptions import DvcException +from dvc.exceptions import DvcException, BadMetricError from dvc.command.base import CmdBase, fix_subparsers, append_doc_link @@ -17,6 +16,10 @@ def show_metrics(metrics, all_branches=False, all_tags=False): metrics (list): Where each element is either a `list` if an xpath was specified, otherwise a `str` """ + # When `metrics` contains a `_missing` key, it means that some files + # specified as `targets` in `repo.metrics.show` didn't contain any metrics. + missing = metrics.pop("_missing", None) + for branch, val in metrics.items(): if all_branches or all_tags: logger.info("{branch}:".format(branch=branch)) @@ -33,6 +36,9 @@ def show_metrics(metrics, all_branches=False, all_tags=False): else: logger.info("\t{}: {}".format(fname, metric)) + if missing: + raise BadMetricError(missing) + class CmdMetricsShow(CmdBase): def run(self): @@ -47,24 +53,6 @@ class CmdMetricsShow(CmdBase): ) show_metrics(metrics, self.args.all_branches, self.args.all_tags) - - found = set( - itertools.chain.from_iterable( - metric.keys() for metric in metrics.values() - ) - ) - - missing = set(self.args.targets) - found - - if missing: - logger.error( - "the following metrics do not exists, " - "are not metric files or are malformed: {paths}".format( - paths=", ".join("'{}'".format(x) for x in missing) - ) - ) - return 1 - except DvcException: logger.exception("failed to show metrics") return 1 diff --git a/dvc/exceptions.py b/dvc/exceptions.py index e8fd2df29..bddf83ec9 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -195,6 +195,16 @@ class ReproductionError(DvcException): super(ReproductionError, self).__init__(msg, cause=ex) +class BadMetricError(DvcException): + def __init__(self, paths): + super(BadMetricError, self).__init__( + "the following metrics do not exists, " + "are not metric files or are malformed: {paths}".format( + paths=", ".join("'{}'".format(path) for path in paths) + ) + ) + + class NoMetricsError(DvcException): def __init__(self): super(NoMetricsError, self).__init__( diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 0facfa8ae..11935e899 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -152,12 +152,11 @@ def _read_metric(fd, typ=None, xpath=None, fname=None, branch=None): return None -def _collect_metrics(repo, targets, recursive, typ, xpath, branch): +def _collect_metrics(repo, path, recursive, typ, xpath, branch): """Gather all the metric outputs. Args: - targets (list): List of paths to metric files or directories. - If there's no target, collect all metrics. + 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, @@ -172,25 +171,20 @@ def _collect_metrics(repo, targets, recursive, typ, xpath, branch): - xpath: """ outs = [out for stage in repo.stages() for out in stage.outs] - found = [] - if targets: - for target in targets: - try: - found += repo.find_outs_by_path( - target, outs=outs, recursive=recursive + 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 ) - except OutputNotFoundError: - logger.debug( - "DVC-file not for found for '{}' in branch '{}'".format( - target, branch - ) - ) - else: - found = outs + ) + return [] res = [] - for o in found: + for o in outs: if not o.metric: continue @@ -273,16 +267,32 @@ def show( recursive=False, ): res = {} + found = set() + + if not targets: + # Iterate once to call `_collect_metrics` on all the stages + targets = [None] for branch in repo.brancher(all_branches=all_branches, all_tags=all_tags): - entries = _collect_metrics( - repo, targets, recursive, typ, xpath, branch - ) - metrics = _read_metrics(repo, entries, branch) + metrics = {} + + for p in targets: + entries = _collect_metrics(repo, p, recursive, typ, xpath, branch) + metric = _read_metrics(repo, entries, branch) + + if metric: + found.add(p) + metrics.update(metric) + if metrics: res[branch] = metrics - if not res and not targets: + if not res and not any(targets): raise NoMetricsError() + missing = set(targets) - found + + if missing: + res["_missing"] = missing + return res diff --git a/tests/func/test_metrics.py b/tests/func/test_metrics.py index 46440ffdf..9d0ba2c70 100644 --- a/tests/func/test_metrics.py +++ b/tests/func/test_metrics.py @@ -415,7 +415,7 @@ class TestMetricsRecursive(TestDvcGit): ret = self.dvc.metrics.show( ["nested"], all_branches=True, recursive=False ) - self.assertEqual(len(ret), 0) + self.assertEqual(len(ret), 1) ret = self.dvc.metrics.show( ["nested"], all_branches=True, recursive=True