diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index d1e019521..48c0bac52 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -3,7 +3,7 @@ from __future__ import unicode_literals import argparse import logging -from dvc.exceptions import DvcException +from dvc.exceptions import DvcException, BadMetricError from dvc.command.base import CmdBase, fix_subparsers, append_doc_link @@ -16,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 `None` key, it means that some files + # specified as `targets` in `repo.metrics.show` didn't contain any metrics. + missing = metrics.pop(None, None) + for branch, val in metrics.items(): if all_branches or all_tags: logger.info("{branch}:".format(branch=branch)) @@ -32,16 +36,17 @@ 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): - typ = self.args.type - xpath = self.args.xpath try: metrics = self.repo.metrics.show( - self.args.path, - typ=typ, - xpath=xpath, + self.args.targets, + typ=self.args.type, + xpath=self.args.xpath, all_branches=self.args.all_branches, all_tags=self.args.all_tags, recursive=self.args.recursive, @@ -121,7 +126,10 @@ def add_parser(subparsers, parent_parser): formatter_class=argparse.RawDescriptionHelpFormatter, ) metrics_show_parser.add_argument( - "path", nargs="?", help="Path to a metric file or a directory." + "targets", + nargs="*", + help="Metric files or directories (see -R) to show " + "(leave empty to display all)", ) metrics_show_parser.add_argument( "-t", @@ -155,8 +163,8 @@ def add_parser(subparsers, parent_parser): action="store_true", default=False, help=( - "If path is a directory, recursively search and process metric " - "files in path." + "If any target is a directory, recursively search and process " + "metric files." ), ) metrics_show_parser.set_defaults(func=CmdMetricsShow) diff --git a/dvc/exceptions.py b/dvc/exceptions.py index da96fd0f9..bddf83ec9 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -196,10 +196,11 @@ class ReproductionError(DvcException): class BadMetricError(DvcException): - def __init__(self, path): + def __init__(self, paths): super(BadMetricError, self).__init__( - "'{}' does not exist, not a metric or is malformed".format( - relpath(path) + "the following metrics do not exists, " + "are not metric files or are malformed: {paths}".format( + paths=", ".join("'{}'".format(path) for path in paths) ) ) diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 42ceab3d0..cd1ab7459 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -8,7 +8,7 @@ import logging from jsonpath_ng.ext import parse -from dvc.exceptions import OutputNotFoundError, BadMetricError, NoMetricsError +from dvc.exceptions import OutputNotFoundError, NoMetricsError from dvc.utils.compat import builtin_str, open, StringIO, csv_reader NO_METRICS_FILE_AT_REFERENCE_WARNING = ( @@ -159,8 +159,8 @@ def _collect_metrics(repo, path, recursive, typ, xpath, branch): 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 of metric to search for, could be one of the - following (raw|json|tsv|htsv|csv|hcsv). + 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. @@ -259,7 +259,7 @@ def _read_metrics(repo, metrics, branch): def show( repo, - path=None, + targets=None, typ=None, xpath=None, all_branches=False, @@ -267,16 +267,34 @@ 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, path, recursive, typ, xpath, branch) - metrics = _read_metrics(repo, entries, branch) + metrics = {} + + for target in targets: + entries = _collect_metrics( + repo, target, recursive, typ, xpath, branch + ) + metric = _read_metrics(repo, entries, branch) + + if metric: + found.add(target) + metrics.update(metric) + if metrics: res[branch] = metrics - if not res: - if path: - raise BadMetricError(path) + if not res and not any(targets): raise NoMetricsError() + missing = set(targets) - found + + if missing: + res[None] = missing + return res diff --git a/tests/func/test_metrics.py b/tests/func/test_metrics.py index e7892f506..9f0b2a00f 100644 --- a/tests/func/test_metrics.py +++ b/tests/func/test_metrics.py @@ -7,7 +7,7 @@ import logging from dvc.repo import Repo as DvcRepo from dvc.main import main -from dvc.exceptions import DvcException, BadMetricError, NoMetricsError +from dvc.exceptions import DvcException, NoMetricsError from dvc.repo.metrics.show import NO_METRICS_FILE_AT_REFERENCE_WARNING from dvc.stage import Stage from dvc.utils import relpath @@ -85,14 +85,14 @@ class TestMetricsBase(TestDvcGit): class TestMetrics(TestMetricsBase): def test_show(self): - ret = self.dvc.metrics.show("metric", all_branches=True) + ret = self.dvc.metrics.show(["metric"], all_branches=True) self.assertEqual(len(ret), 3) self.assertEqual(ret["foo"]["metric"], "foo") self.assertEqual(ret["bar"]["metric"], "bar") self.assertEqual(ret["baz"]["metric"], "baz") ret = self.dvc.metrics.show( - "metric_json", typ="json", xpath="branch", all_branches=True + ["metric_json"], typ="json", xpath="branch", all_branches=True ) self.assertEqual(len(ret), 3) self.assertSequenceEqual(ret["foo"]["metric_json"], ["foo"]) @@ -100,7 +100,7 @@ class TestMetrics(TestMetricsBase): self.assertSequenceEqual(ret["baz"]["metric_json"], ["baz"]) ret = self.dvc.metrics.show( - "metric_tsv", typ="tsv", xpath="0,0", all_branches=True + ["metric_tsv"], typ="tsv", xpath="0,0", all_branches=True ) self.assertEqual(len(ret), 3) self.assertSequenceEqual(ret["foo"]["metric_tsv"], ["foo"]) @@ -108,7 +108,7 @@ class TestMetrics(TestMetricsBase): self.assertSequenceEqual(ret["baz"]["metric_tsv"], ["baz"]) ret = self.dvc.metrics.show( - "metric_htsv", typ="htsv", xpath="0,branch", all_branches=True + ["metric_htsv"], typ="htsv", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) self.assertSequenceEqual(ret["foo"]["metric_htsv"], ["foo"]) @@ -116,7 +116,7 @@ class TestMetrics(TestMetricsBase): self.assertSequenceEqual(ret["baz"]["metric_htsv"], ["baz"]) ret = self.dvc.metrics.show( - "metric_csv", typ="csv", xpath="0,0", all_branches=True + ["metric_csv"], typ="csv", xpath="0,0", all_branches=True ) self.assertEqual(len(ret), 3) self.assertSequenceEqual(ret["foo"]["metric_csv"], ["foo"]) @@ -124,7 +124,7 @@ class TestMetrics(TestMetricsBase): self.assertSequenceEqual(ret["baz"]["metric_csv"], ["baz"]) ret = self.dvc.metrics.show( - "metric_hcsv", typ="hcsv", xpath="0,branch", all_branches=True + ["metric_hcsv"], typ="hcsv", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) self.assertSequenceEqual(ret["foo"]["metric_hcsv"], ["foo"]) @@ -132,7 +132,7 @@ class TestMetrics(TestMetricsBase): self.assertSequenceEqual(ret["baz"]["metric_hcsv"], ["baz"]) ret = self.dvc.metrics.show( - "metric_json_ext", + ["metric_json_ext"], typ="json", xpath="$.metrics[?(@.deviation_mse<0.30) & (@.value_mse>0.4)]", all_branches=True, @@ -153,7 +153,7 @@ class TestMetrics(TestMetricsBase): def test_unknown_type_ignored(self): ret = self.dvc.metrics.show( - "metric_hcsv", typ="unknown", xpath="0,branch", all_branches=True + ["metric_hcsv"], typ="unknown", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -161,7 +161,7 @@ class TestMetrics(TestMetricsBase): def test_type_case_normalized(self): ret = self.dvc.metrics.show( - "metric_hcsv", typ=" hCSV ", xpath="0,branch", all_branches=True + ["metric_hcsv"], typ=" hCSV ", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -169,7 +169,7 @@ class TestMetrics(TestMetricsBase): def test_xpath_is_empty(self): ret = self.dvc.metrics.show( - "metric_json", typ="json", xpath="", all_branches=True + ["metric_json"], typ="json", xpath="", all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -177,7 +177,7 @@ class TestMetrics(TestMetricsBase): def test_xpath_is_none(self): ret = self.dvc.metrics.show( - "metric_json", typ="json", xpath=None, all_branches=True + ["metric_json"], typ="json", xpath=None, all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -185,7 +185,7 @@ class TestMetrics(TestMetricsBase): def test_xpath_all_columns(self): ret = self.dvc.metrics.show( - "metric_hcsv", typ="hcsv ", xpath="0,", all_branches=True + ["metric_hcsv"], typ="hcsv ", xpath="0,", all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -193,7 +193,7 @@ class TestMetrics(TestMetricsBase): def test_xpath_all_rows(self): ret = self.dvc.metrics.show( - "metric_csv", typ="csv", xpath=",0", all_branches=True + ["metric_csv"], typ="csv", xpath=",0", all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -201,7 +201,7 @@ class TestMetrics(TestMetricsBase): def test_xpath_all(self): ret = self.dvc.metrics.show( - "metric_csv", typ="csv", xpath=",", all_branches=True + ["metric_csv"], typ="csv", xpath=",", all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -209,7 +209,7 @@ class TestMetrics(TestMetricsBase): def test_xpath_all_with_header(self): ret = self.dvc.metrics.show( - "metric_hcsv", typ="hcsv", xpath=",", all_branches=True + ["metric_hcsv"], typ="hcsv", xpath=",", all_branches=True ) self.assertEqual(len(ret), 3) for b in ["foo", "bar", "baz"]: @@ -412,11 +412,13 @@ class TestMetricsRecursive(TestDvcGit): self.dvc.scm.checkout("master") def test(self): - with self.assertRaises(BadMetricError): - self.dvc.metrics.show("nested", all_branches=True, recursive=False) + ret = self.dvc.metrics.show( + ["nested"], all_branches=True, recursive=False + ) + self.assertEqual(len(ret), 1) ret = self.dvc.metrics.show( - "nested", all_branches=True, recursive=True + ["nested"], all_branches=True, recursive=True ) self.assertEqual(len(ret), 1) self.assertEqual( @@ -697,7 +699,7 @@ class TestCachedMetrics(TestDvcGit): self.dvc = DvcRepo(".") res = self.dvc.metrics.show( - "metrics.json", all_branches=True, typ="json", xpath="metrics" + ["metrics.json"], all_branches=True, typ="json", xpath="metrics" ) self.assertEqual( @@ -711,7 +713,7 @@ class TestCachedMetrics(TestDvcGit): ) res = self.dvc.metrics.show( - "", all_branches=True, typ="json", xpath="metrics" + all_branches=True, typ="json", xpath="metrics" ) self.assertEqual( @@ -777,7 +779,9 @@ class TestMetricsType(TestDvcGit): self._do_show(file_name, xpath) def _do_show(self, file_name, xpath): - ret = self.dvc.metrics.show(file_name, xpath=xpath, all_branches=True) + ret = self.dvc.metrics.show( + [file_name], xpath=xpath, all_branches=True + ) self.assertEqual(len(ret), 3) for branch in self.branches: if isinstance(ret[branch][file_name], list): @@ -844,3 +848,41 @@ def test_show_xpath_should_override_stage_xpath(dvc_repo): result = dvc_repo.metrics.show(xpath="m1") assert result == {"": {metric_file: [0.1]}} + + +def test_show_multiple_outputs(dvc_repo, caplog): + with open("1.json", "w") as fobj: + json.dump({"AUC": 1}, fobj) + + with open("2.json", "w") as fobj: + json.dump({"AUC": 2}, fobj) + + os.mkdir("metrics") + with open("metrics/3.json", "w") as fobj: + json.dump({"AUC": 3}, fobj) + + dvc_repo.run(cmd="", overwrite=True, metrics=["1.json"]) + dvc_repo.run(cmd="", overwrite=True, metrics=["2.json"]) + dvc_repo.run(cmd="", overwrite=True, metrics=["metrics/3.json"]) + + with caplog.at_level(logging.INFO, logger="dvc"): + assert 0 == main(["metrics", "show", "1.json", "2.json"]) + assert '1.json: {"AUC": 1}' in caplog.text + assert '2.json: {"AUC": 2}' in caplog.text + + caplog.clear() + + with caplog.at_level(logging.INFO, logger="dvc"): + assert 0 == main(["metrics", "show", "-R", "1.json", "metrics"]) + assert '1.json: {"AUC": 1}' in caplog.text + assert '3.json: {"AUC": 3}' in caplog.text + + caplog.clear() + + with caplog.at_level(logging.INFO, logger="dvc"): + assert 1 == main(["metrics", "show", "1.json", "not-found"]) + assert '1.json: {"AUC": 1}' in caplog.text + assert ( + "the following metrics do not exists, " + "are not metric files or are malformed: 'not-found'" + ) in caplog.text