From bd382c87d90826113b0166e9cb5e2e7ec9a6d7ba Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Sun, 17 Feb 2019 15:08:58 -0800 Subject: [PATCH 1/3] cleanup metrics help, fix type and xpath handling --- dvc/command/metrics.py | 74 ++++++++++--------------------------- dvc/logger.py | 13 ++++++- dvc/output/base.py | 13 ++++++- dvc/output/local.py | 4 +- dvc/repo/metrics/modify.py | 12 +++--- dvc/repo/metrics/show.py | 21 +++++------ tests/test_metrics.py | 75 ++------------------------------------ 7 files changed, 65 insertions(+), 147 deletions(-) diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index ce014e436..bc7f6f5e7 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -14,31 +14,9 @@ class CmdMetricsShow(CmdBase): for fname, metric in val.items(): logger.info("\t{}: {}".format(fname, metric)) - def _get_type_xpath(self): - # backward compatibility - if self.args.json_path: - typ = "json" - xpath = self.args.json_path - elif self.args.tsv_path: - typ = "tsv" - xpath = self.args.tsv_path - elif self.args.htsv_path: - typ = "htsv" - xpath = self.args.htsv_path - elif self.args.csv_path: - typ = "csv" - xpath = self.args.csv_path - elif self.args.hcsv_path: - typ = "hcsv" - xpath = self.args.hcsv_path - else: - typ = self.args.type - xpath = self.args.xpath - - return typ, xpath - def run(self): - typ, xpath = self._get_type_xpath() + typ = self.args.type + xpath = self.args.xpath try: metrics = self.repo.metrics.show( self.args.path, @@ -64,7 +42,7 @@ class CmdMetricsModify(CmdBase): self.args.path, typ=self.args.type, xpath=self.args.xpath ) except DvcException: - logger.error("failed to modify metrics") + logger.error("failed to modify metric file settings") return 1 return 0 @@ -97,7 +75,7 @@ class CmdMetricsRemove(CmdBase): def add_parser(subparsers, parent_parser): - METRICS_HELP = "Get metrics from all branches." + METRICS_HELP = "A set of commands to add, manage, collect and display project metrics." metrics_parser = subparsers.add_parser( "metrics", parents=[parent_parser], @@ -107,12 +85,12 @@ def add_parser(subparsers, parent_parser): metrics_subparsers = metrics_parser.add_subparsers( dest="cmd", - help="Use dvc metrics CMD --help for command-specific help.", + help="Use dvc metrics CMD --help to display command-specific help.", ) fix_subparsers(metrics_subparsers) - METRICS_SHOW_HELP = "Show metrics." + METRICS_SHOW_HELP = "Output metric values." metrics_show_parser = metrics_subparsers.add_parser( "show", parents=[parent_parser], @@ -120,27 +98,13 @@ def add_parser(subparsers, parent_parser): help=METRICS_SHOW_HELP, ) metrics_show_parser.add_argument( - "path", nargs="?", help="Path to metrics file or directory" + "path", nargs="?", help="Path to a metric file or a directory." ) metrics_show_parser.add_argument( - "-t", "--type", help="Type of metrics(RAW/JSON/TSV/HTSV/CSV/HCSV)." + "-t", "--type", help="Type of metrics (raw/json/tsv/htsv/csv/hcsv)." ) metrics_show_parser.add_argument( - "-x", "--xpath", help="JSON/TSV/HTSV/CSV/HCSV path." - ) - metrics_show_group = metrics_show_parser.add_mutually_exclusive_group() - metrics_show_group.add_argument("--json-path", help="JSON path.") - metrics_show_group.add_argument( - "--tsv-path", help="TSV path 'row,column' (e.g. '1,2')." - ) - metrics_show_group.add_argument( - "--htsv-path", help="Headed TSV path 'row,column (e.g. 'Name,3')." - ) - metrics_show_group.add_argument( - "--csv-path", help="CSV path 'row,column' (e.g. '1,2')." - ) - metrics_show_group.add_argument( - "--hcsv-path", help="Headed CSV path 'row,column' (e.g. 'Name,3')." + "-x", "--xpath", help="json/tsv/htsv/csv/hcsv path." ) metrics_show_parser.add_argument( "-a", @@ -165,7 +129,7 @@ def add_parser(subparsers, parent_parser): ) metrics_show_parser.set_defaults(func=CmdMetricsShow) - METRICS_ADD_HELP = "Add metrics." + METRICS_ADD_HELP = "Tag file as a metric file." metrics_add_parser = metrics_subparsers.add_parser( "add", parents=[parent_parser], @@ -173,15 +137,15 @@ def add_parser(subparsers, parent_parser): help=METRICS_ADD_HELP, ) metrics_add_parser.add_argument( - "-t", "--type", help="Type of metrics(RAW/JSON/TSV/HTSV/CSV/HCSV)." + "-t", "--type", help="Type of metrics (raw/json/tsv/htsv/csv/hcsv)." ) metrics_add_parser.add_argument( - "-x", "--xpath", help="JSON/TSV/HTSV/CSV/HCSV path." + "-x", "--xpath", help="json/tsv/htsv/csv/hcsv path." ) - metrics_add_parser.add_argument("path", help="Path to metrics file.") + metrics_add_parser.add_argument("path", help="Path to a metric file.") metrics_add_parser.set_defaults(func=CmdMetricsAdd) - METRICS_MODIFY_HELP = "Modify metrics." + METRICS_MODIFY_HELP = "Modify metric file options." metrics_modify_parser = metrics_subparsers.add_parser( "modify", parents=[parent_parser], @@ -189,20 +153,20 @@ def add_parser(subparsers, parent_parser): help=METRICS_MODIFY_HELP, ) metrics_modify_parser.add_argument( - "-t", "--type", help="Type of metrics(RAW/JSON/TSV/HTSV/CSV/HCSV)." + "-t", "--type", help="Type of metrics (raw/json/tsv/htsv/csv/hcsv)." ) metrics_modify_parser.add_argument( - "-x", "--xpath", help="JSON/TSV/HTSV/CSV/HCSV path." + "-x", "--xpath", help="json/tsv/htsv/csv/hcsv path." ) - metrics_modify_parser.add_argument("path", help="Metrics file.") + metrics_modify_parser.add_argument("path", help="Path to a metric file.") metrics_modify_parser.set_defaults(func=CmdMetricsModify) - METRICS_REMOVE_HELP = "Remove metrics." + METRICS_REMOVE_HELP = "Remove files's metric tag." metrics_remove_parser = metrics_subparsers.add_parser( "remove", parents=[parent_parser], description=METRICS_REMOVE_HELP, help=METRICS_REMOVE_HELP, ) - metrics_remove_parser.add_argument("path", help="Path to metrics file.") + metrics_remove_parser.add_argument("path", help="Path to a metric file.") metrics_remove_parser.set_defaults(func=CmdMetricsRemove) diff --git a/dvc/logger.py b/dvc/logger.py index 2836bcffa..abc02aed4 100644 --- a/dvc/logger.py +++ b/dvc/logger.py @@ -29,11 +29,20 @@ def debug(message): logger.debug(out) -def warning(message): +def warning(message, parse_exception=False): """Prints a warning message.""" prefix = colorize("Warning", color="yellow") - out = "{prefix}: {message}".format(prefix=prefix, message=message) + exception, stack_trace = None, "" + if parse_exception: + exception, stack_trace = _parse_exc() + + out = "{prefix}: {description}".format( + prefix=prefix, description=_description(message, exception) + ) + + if stack_trace: + out += "\n{stack_trace}".format(stack_trace=stack_trace) logger.warning(out) diff --git a/dvc/output/base.py b/dvc/output/base.py index 9fb8bb1ca..c956e89c9 100644 --- a/dvc/output/base.py +++ b/dvc/output/base.py @@ -1,10 +1,10 @@ from __future__ import unicode_literals -from dvc.utils.compat import str import re from schema import Or, Optional from dvc.exceptions import DvcException +from dvc.utils.compat import str class OutputDoesNotExistError(DvcException): @@ -77,6 +77,12 @@ class OutputBase(object): def __str__(self): return self.url + @property + def is_local(self): + raise DvcException( + "is local is not supported for {}".format(self.scheme) + ) + @classmethod def match(cls, url): return re.match(cls.REMOTE.REGEX, url) @@ -174,6 +180,11 @@ class OutputBase(object): return ret + def verify_metric(self): + raise DvcException( + "verify metric is not supported for {}".format(self.scheme) + ) + def download(self, to_info, resume=False): self.remote.download([self.path_info], [to_info], resume=resume) diff --git a/dvc/output/local.py b/dvc/output/local.py index 142d18e16..c330e09ea 100644 --- a/dvc/output/local.py +++ b/dvc/output/local.py @@ -67,7 +67,7 @@ class OutputLOCAL(OutputBase): return ret - def _verify_metric(self): + def verify_metric(self): if not self.metric: return @@ -97,7 +97,7 @@ class OutputLOCAL(OutputBase): if not self.use_cache: self.info = self.remote.save_info(self.path_info) - self._verify_metric() + self.verify_metric() if not self.IS_DEPENDENCY: msg = "Output '{}' doesn't use cache. Skipping saving." logger.info(msg.format(self.rel_path)) diff --git a/dvc/repo/metrics/modify.py b/dvc/repo/metrics/modify.py index f26b6ff62..c28c8bfd8 100644 --- a/dvc/repo/metrics/modify.py +++ b/dvc/repo/metrics/modify.py @@ -1,7 +1,5 @@ from __future__ import unicode_literals -import os - from dvc.exceptions import DvcException @@ -14,12 +12,16 @@ 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: + if typ is not None: + typ = typ.lower().strip() + if typ not in ["raw", "json", "csv", "tsv", "hcsv", "htsv"]: + msg = "metric type '{}' is not supported" + raise DvcException(msg.format(typ)) if not isinstance(out.metric, dict): out.metric = {} out.metric[out.PARAM_METRIC_TYPE] = typ - if xpath: + if xpath is not None: if not isinstance(out.metric, dict): out.metric = {} out.metric[out.PARAM_METRIC_XPATH] = xpath @@ -27,6 +29,6 @@ def modify(repo, path, typ=None, xpath=None, delete=False): if delete: out.metric = None - out._verify_metric() + out.verify_metric() out.stage.dump() diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 1024c89c0..d86391bb5 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -6,14 +6,8 @@ import json from jsonpath_rw import parse import dvc.logger as logger -from dvc.exceptions import ( - OutputDuplicationError, - DvcException, - OutputNotFoundError, - BadMetricError, - NoMetricsError, -) -from dvc.utils.compat import str, builtin_str, open +from dvc.exceptions import OutputNotFoundError, BadMetricError, NoMetricsError +from dvc.utils.compat import builtin_str, open def _read_metric_json(fd, json_path): @@ -32,14 +26,14 @@ def _do_read_metric_xsv(reader, row, col): def _read_metric_hxsv(fd, hxsv_path, delimiter): - col, row = hxsv_path.split(",") + row, col = hxsv_path.split(",") row = int(row) reader = list(csv.DictReader(fd, delimiter=builtin_str(delimiter))) return _do_read_metric_xsv(reader, row, col) def _read_metric_xsv(fd, xsv_path, delimiter): - col, row = xsv_path.split(",") + row, col = xsv_path.split(",") row = int(row) col = int(col) reader = list(csv.reader(fd, delimiter=builtin_str(delimiter))) @@ -52,6 +46,7 @@ def _read_metric(path, typ=None, xpath=None): if not os.path.exists(path): return ret + typ = typ.lower().strip() if typ else typ try: with open(path, "r") as fd: if typ == "json": @@ -66,8 +61,12 @@ def _read_metric(path, typ=None, xpath=None): ret = _read_metric_hxsv(fd, xpath, "\t") else: ret = fd.read().strip() + # Json path library has to be replaced or wrapped in + # order to fix this too broad except clause. except Exception: - logger.error("unable to read metric in '{}'".format(path)) + logger.warning( + "unable to read metric in '{}'".format(path), parse_exception=True + ) return ret diff --git a/tests/test_metrics.py b/tests/test_metrics.py index 8409850ef..97374a198 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -76,7 +76,7 @@ class TestMetrics(TestDvc): self.assertTrue(ret["baz"]["metric_tsv"] == ["baz"]) ret = self.dvc.metrics.show( - "metric_htsv", typ="htsv", xpath="branch,0", all_branches=True + "metric_htsv", typ="htsv", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) self.assertTrue(ret["foo"]["metric_htsv"] == ["foo"]) @@ -92,7 +92,7 @@ class TestMetrics(TestDvc): self.assertTrue(ret["baz"]["metric_csv"] == ["baz"]) ret = self.dvc.metrics.show( - "metric_hcsv", typ="hcsv", xpath="branch,0", all_branches=True + "metric_hcsv", typ="hcsv", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) self.assertTrue(ret["foo"]["metric_hcsv"] == ["foo"]) @@ -203,73 +203,6 @@ class TestMetricsReproCLI(TestDvc): self.dvc.run(metrics_no_cache=["metrics_bin"]) -class TestMetricsCLICompat(TestMetrics): - def test(self): - # FIXME check output - ret = main(["metrics", "show", "--all-branches", "metric", "-v"]) - self.assertEqual(ret, 0) - - ret = main( - [ - "metrics", - "show", - "--all-branches", - "metric_json", - "--json-path", - "branch", - ] - ) - self.assertEqual(ret, 0) - - ret = main( - [ - "metrics", - "show", - "--all-branches", - "metric_tsv", - "--tsv-path", - "0,0", - ] - ) - self.assertEqual(ret, 0) - - ret = main( - [ - "metrics", - "show", - "--all-branches", - "metric_htsv", - "--htsv-path", - "branch,0", - ] - ) - self.assertEqual(ret, 0) - - ret = main( - [ - "metrics", - "show", - "--all-branches", - "metric_csv", - "--csv-path", - "0,0", - ] - ) - self.assertEqual(ret, 0) - - ret = main( - [ - "metrics", - "show", - "--all-branches", - "metric_hcsv", - "--hcsv-path", - "branch,0", - ] - ) - self.assertEqual(ret, 0) - - class TestMetricsCLI(TestMetrics): def test(self): # FIXME check output @@ -304,7 +237,7 @@ class TestMetricsCLI(TestMetrics): "-t", "htsv", "-x", - "branch,0", + "0,branch", ] ) self.assertEqual(ret, 0) @@ -323,7 +256,7 @@ class TestMetricsCLI(TestMetrics): "-t", "hcsv", "-x", - "branch,0", + "0,branch", ] ) self.assertEqual(ret, 0) From 708558da628e6b259807bd6ad8f7ad2288cb11c5 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Sun, 17 Feb 2019 15:24:46 -0800 Subject: [PATCH 2/3] metrics tests: add test for wrong metric type, uppercase type --- tests/test_metrics.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_metrics.py b/tests/test_metrics.py index 97374a198..d3172bdd0 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -68,7 +68,7 @@ class TestMetrics(TestDvc): self.assertTrue(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.assertTrue(ret["foo"]["metric_tsv"] == ["foo"]) @@ -84,7 +84,7 @@ class TestMetrics(TestDvc): self.assertTrue(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.assertTrue(ret["foo"]["metric_csv"] == ["foo"]) @@ -145,9 +145,7 @@ class TestMetricsRecursive(TestDvc): def test(self): with self.assertRaises(BadMetricError): - ret = self.dvc.metrics.show( - "nested", all_branches=True, recursive=False - ) + self.dvc.metrics.show("nested", all_branches=True, recursive=False) ret = self.dvc.metrics.show( "nested", all_branches=True, recursive=True @@ -183,7 +181,10 @@ class TestMetricsReproCLI(TestDvc): ret = main(["metrics", "add", "metrics"]) self.assertEqual(ret, 0) - ret = main(["metrics", "modify", "-t", "csv", "-x", "0,0", "metrics"]) + ret = main(["metrics", "modify", "-t", "XSV", "-x", "0,0", "metrics"]) + self.assertEqual(ret, 1) + + ret = main(["metrics", "modify", "-t", "CSV", "-x", "0,0", "metrics"]) self.assertEqual(ret, 0) ret = main(["repro", "-f", "-m", stage.path]) @@ -243,7 +244,7 @@ class TestMetricsCLI(TestMetrics): self.assertEqual(ret, 0) ret = main( - ["metrics", "show", "-a", "metric_csv", "-t", "csv", "-x", "0,0"] + ["metrics", "show", "-a", "metric_csv", "-t", "CSV", "-x", "0,0"] ) self.assertEqual(ret, 0) From b79085ec6901fede910ac0da6a1caeff2de006e5 Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Mon, 18 Feb 2019 17:17:20 -0800 Subject: [PATCH 3/3] add tests for metrics wrong type and some other edge cases --- dvc/repo/__init__.py | 3 +- dvc/repo/metrics/modify.py | 7 +- dvc/repo/metrics/show.py | 78 ++++++++++----- tests/test_metrics.py | 190 +++++++++++++++++++++++++++++++------ 4 files changed, 221 insertions(+), 57 deletions(-) diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 889abc768..025bfd2ef 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -1,8 +1,7 @@ from __future__ import unicode_literals -from dvc.utils.compat import str, builtin_str, open +from dvc.utils.compat import str -import collections import os import dvc.prompt as prompt import dvc.logger as logger diff --git a/dvc/repo/metrics/modify.py b/dvc/repo/metrics/modify.py index c28c8bfd8..541446b6a 100644 --- a/dvc/repo/metrics/modify.py +++ b/dvc/repo/metrics/modify.py @@ -4,6 +4,7 @@ from dvc.exceptions import DvcException def modify(repo, path, typ=None, xpath=None, delete=False): + supported_types = ["raw", "json", "csv", "tsv", "hcsv", "htsv"] outs = repo.find_outs_by_path(path) assert len(outs) == 1 out = outs[0] @@ -15,8 +16,10 @@ def modify(repo, path, typ=None, xpath=None, delete=False): if typ is not None: typ = typ.lower().strip() if typ not in ["raw", "json", "csv", "tsv", "hcsv", "htsv"]: - msg = "metric type '{}' is not supported" - raise DvcException(msg.format(typ)) + 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 diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index d86391bb5..f273d2c22 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -15,69 +15,95 @@ def _read_metric_json(fd, json_path): return [x.value for x in parser.find(json.load(fd))] +def _get_values(row): + if isinstance(row, dict): + return list(row.values()) + else: + return row + + def _do_read_metric_xsv(reader, row, col): if col is not None and row is not None: return [reader[row][col]] elif col is not None: return [r[col] for r in reader] elif row is not None: - return reader[row] - return None + return _get_values(reader[row]) + return [_get_values(r) for r in reader] def _read_metric_hxsv(fd, hxsv_path, delimiter): - row, col = hxsv_path.split(",") - row = int(row) + 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=builtin_str(delimiter))) return _do_read_metric_xsv(reader, row, col) def _read_metric_xsv(fd, xsv_path, delimiter): - row, col = xsv_path.split(",") - row = int(row) - col = int(col) + 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=builtin_str(delimiter))) return _do_read_metric_xsv(reader, row, col) -def _read_metric(path, typ=None, xpath=None): - ret = None +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 _read_metric(path, typ=None, xpath=None, branch=None): + ret = None if not os.path.exists(path): return ret typ = typ.lower().strip() if typ else typ + xpath = xpath.strip() if xpath else xpath try: with open(path, "r") as 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: + if not xpath: ret = fd.read().strip() + else: + ret = _read_typed_metric(typ, xpath, fd) # Json path library has to be replaced or wrapped in # order to fix this too broad except clause. except Exception: logger.warning( - "unable to read metric in '{}'".format(path), parse_exception=True + "unable to read metric in '{}' in branch '{}'".format( + path, branch + ), + parse_exception=True, ) return ret -def _collect_metrics(self, path, recursive, typ, xpath): +def _collect_metrics(self, path, recursive, typ, xpath, branch): outs = [out for stage in self.stages() for out in stage.outs] if path: try: outs = self.find_outs_by_path(path, outs=outs, recursive=recursive) except OutputNotFoundError: + logger.debug( + "stage file not for found for '{}' in branch '{}'".format( + path, branch + ) + ) return [] res = [] @@ -97,7 +123,7 @@ def _collect_metrics(self, path, recursive, typ, xpath): return res -def _read_metrics(self, metrics): +def _read_metrics(self, metrics, branch): res = {} for out, typ, xpath in metrics: assert out.scheme == "local" @@ -106,7 +132,7 @@ def _read_metrics(self, metrics): else: path = out.path - metric = _read_metric(path, typ=typ, xpath=xpath) + metric = _read_metric(path, typ=typ, xpath=xpath, branch=branch) if not metric: continue @@ -128,8 +154,8 @@ def show( for branch in self.scm.brancher( all_branches=all_branches, all_tags=all_tags ): - entries = _collect_metrics(self, path, recursive, typ, xpath) - metrics = _read_metrics(self, entries) + entries = _collect_metrics(self, path, recursive, typ, xpath, branch) + metrics = _read_metrics(self, entries, branch) if metrics: res[branch] = metrics diff --git a/tests/test_metrics.py b/tests/test_metrics.py index d3172bdd0..169707897 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -6,6 +6,9 @@ from dvc.main import main from dvc.exceptions import DvcException, BadMetricError, NoMetricsError from tests.basic_env import TestDvc +import dvc.logger as logger +from dvc.utils.compat import StringIO + class TestMetrics(TestDvc): def setUp(self): @@ -52,52 +55,116 @@ class TestMetrics(TestDvc): self.dvc.scm.checkout("master") - def test(self): + def test_show(self): ret = self.dvc.metrics.show("metric", all_branches=True) self.assertEqual(len(ret), 3) - self.assertTrue(ret["foo"]["metric"] == "foo") - self.assertTrue(ret["bar"]["metric"] == "bar") - self.assertTrue(ret["baz"]["metric"] == "baz") + 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 ) self.assertEqual(len(ret), 3) - self.assertTrue(ret["foo"]["metric_json"] == ["foo"]) - self.assertTrue(ret["bar"]["metric_json"] == ["bar"]) - self.assertTrue(ret["baz"]["metric_json"] == ["baz"]) + self.assertSequenceEqual(ret["foo"]["metric_json"], ["foo"]) + self.assertSequenceEqual(ret["bar"]["metric_json"], ["bar"]) + 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.assertTrue(ret["foo"]["metric_tsv"] == ["foo"]) - self.assertTrue(ret["bar"]["metric_tsv"] == ["bar"]) - self.assertTrue(ret["baz"]["metric_tsv"] == ["baz"]) + self.assertSequenceEqual(ret["foo"]["metric_tsv"], ["foo"]) + self.assertSequenceEqual(ret["bar"]["metric_tsv"], ["bar"]) + self.assertSequenceEqual(ret["baz"]["metric_tsv"], ["baz"]) ret = self.dvc.metrics.show( "metric_htsv", typ="htsv", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) - self.assertTrue(ret["foo"]["metric_htsv"] == ["foo"]) - self.assertTrue(ret["bar"]["metric_htsv"] == ["bar"]) - self.assertTrue(ret["baz"]["metric_htsv"] == ["baz"]) + self.assertSequenceEqual(ret["foo"]["metric_htsv"], ["foo"]) + self.assertSequenceEqual(ret["bar"]["metric_htsv"], ["bar"]) + 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.assertTrue(ret["foo"]["metric_csv"] == ["foo"]) - self.assertTrue(ret["bar"]["metric_csv"] == ["bar"]) - self.assertTrue(ret["baz"]["metric_csv"] == ["baz"]) + self.assertSequenceEqual(ret["foo"]["metric_csv"], ["foo"]) + self.assertSequenceEqual(ret["bar"]["metric_csv"], ["bar"]) + self.assertSequenceEqual(ret["baz"]["metric_csv"], ["baz"]) ret = self.dvc.metrics.show( "metric_hcsv", typ="hcsv", xpath="0,branch", all_branches=True ) self.assertEqual(len(ret), 3) - self.assertTrue(ret["foo"]["metric_hcsv"] == ["foo"]) - self.assertTrue(ret["bar"]["metric_hcsv"] == ["bar"]) - self.assertTrue(ret["baz"]["metric_hcsv"] == ["baz"]) + self.assertSequenceEqual(ret["foo"]["metric_hcsv"], ["foo"]) + self.assertSequenceEqual(ret["bar"]["metric_hcsv"], ["bar"]) + self.assertSequenceEqual(ret["baz"]["metric_hcsv"], ["baz"]) + + def test_unknown_type_ignored(self): + ret = self.dvc.metrics.show( + "metric_hcsv", typ="unknown", xpath="0,branch", all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertSequenceEqual(ret[b]["metric_hcsv"], "branch\n" + b) + + def test_type_case_normalized(self): + ret = self.dvc.metrics.show( + "metric_hcsv", typ=" hCSV ", xpath="0,branch", all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertSequenceEqual(ret[b]["metric_hcsv"], [b]) + + def test_xpath_is_empty(self): + ret = self.dvc.metrics.show( + "metric_json", typ="json", xpath="", all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertEqual(ret[b]["metric_json"], json.dumps({"branch": b})) + + def test_xpath_is_none(self): + ret = self.dvc.metrics.show( + "metric_json", typ="json", xpath=None, all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertEqual(ret[b]["metric_json"], json.dumps({"branch": b})) + + def test_xpath_all_columns(self): + ret = self.dvc.metrics.show( + "metric_hcsv", typ="hcsv ", xpath="0,", all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertSequenceEqual(ret[b]["metric_hcsv"], [b]) + + def test_xpath_all_rows(self): + ret = self.dvc.metrics.show( + "metric_csv", typ="csv", xpath=",0", all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertSequenceEqual(ret[b]["metric_csv"], [b]) + + def test_xpath_all(self): + ret = self.dvc.metrics.show( + "metric_csv", typ="csv", xpath=",", all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertSequenceEqual(ret[b]["metric_csv"], [[b]]) + + def test_xpath_all_with_header(self): + ret = self.dvc.metrics.show( + "metric_hcsv", typ="hcsv", xpath=",", all_branches=True + ) + self.assertEqual(len(ret), 3) + for b in ["foo", "bar", "baz"]: + self.assertSequenceEqual(ret[b]["metric_hcsv"], [[b]]) class TestMetricsRecursive(TestDvc): @@ -181,9 +248,6 @@ class TestMetricsReproCLI(TestDvc): ret = main(["metrics", "add", "metrics"]) self.assertEqual(ret, 0) - ret = main(["metrics", "modify", "-t", "XSV", "-x", "0,0", "metrics"]) - self.assertEqual(ret, 1) - ret = main(["metrics", "modify", "-t", "CSV", "-x", "0,0", "metrics"]) self.assertEqual(ret, 0) @@ -223,12 +287,10 @@ class TestMetricsCLI(TestMetrics): ] ) self.assertEqual(ret, 0) - ret = main( ["metrics", "show", "-a", "metric_tsv", "-t", "tsv", "-x", "0,0"] ) self.assertEqual(ret, 0) - ret = main( [ "metrics", @@ -244,7 +306,7 @@ class TestMetricsCLI(TestMetrics): self.assertEqual(ret, 0) ret = main( - ["metrics", "show", "-a", "metric_csv", "-t", "CSV", "-x", "0,0"] + ["metrics", "show", "-a", "metric_csv", "-t", "csv", "-x", "0,0"] ) self.assertEqual(ret, 0) @@ -287,6 +349,80 @@ class TestMetricsCLI(TestMetrics): ret = main(["metrics", "remove", "non-existing"]) self.assertNotEqual(ret, 0) + def test_wrong_type_add(self): + with open("metric.unknown", "w+") as fd: + fd.write("unknown") + fd.flush() + + ret = main(["add", "metric.unknown"]) + self.assertEqual(ret, 0) + + logger.logger.handlers[1].stream = StringIO() + ret = main(["metrics", "add", "metric.unknown", "-t", "unknown"]) + self.assertEqual(ret, 1) + self.assertIn( + "failed to add metric file 'metric.unknown' - metric type 'unknown'" + " is not supported, must be one of [raw, json, csv, tsv, hcsv, htsv]", + logger.logger.handlers[1].stream.getvalue(), + ) + + ret = main(["metrics", "add", "metric.unknown", "-t", "raw"]) + self.assertEqual(ret, 0) + + logger.logger.handlers[0].stream = StringIO() + ret = main(["metrics", "show", "metric.unknown"]) + self.assertEqual(ret, 0) + self.assertIn( + "\tmetric.unknown: unknown", + logger.logger.handlers[0].stream.getvalue(), + ) + + def test_wrong_type_modify(self): + with open("metric.unknown", "w+") as fd: + fd.write("unknown") + fd.flush() + + ret = main(["run", "-m", "metric.unknown"]) + self.assertEqual(ret, 0) + + logger.logger.handlers[1].stream = StringIO() + ret = main(["metrics", "modify", "metric.unknown", "-t", "unknown"]) + self.assertEqual(ret, 1) + self.assertIn( + "failed to modify metric file settings - metric type 'unknown'" + " is not supported, must be one of [raw, json, csv, tsv, hcsv, htsv]", + logger.logger.handlers[1].stream.getvalue(), + ) + + ret = main(["metrics", "modify", "metric.unknown", "-t", "CSV"]) + self.assertEqual(ret, 0) + + logger.logger.handlers[0].stream = StringIO() + ret = main(["metrics", "show", "metric.unknown"]) + self.assertEqual(ret, 0) + self.assertIn( + "\tmetric.unknown: unknown", + logger.logger.handlers[0].stream.getvalue(), + ) + + def test_wrong_type_show(self): + with open("metric.unknown", "w+") as fd: + fd.write("unknown") + fd.flush() + + ret = main(["run", "-m", "metric.unknown"]) + self.assertEqual(ret, 0) + + logger.logger.handlers[0].stream = StringIO() + ret = main( + ["metrics", "show", "metric.unknown", "-t", "unknown", "-x", "0,0"] + ) + self.assertEqual(ret, 0) + self.assertIn( + "\tmetric.unknown: unknown", + logger.logger.handlers[0].stream.getvalue(), + ) + class TestNoMetrics(TestDvc): def test(self):