From bd382c87d90826113b0166e9cb5e2e7ec9a6d7ba Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Sun, 17 Feb 2019 15:08:58 -0800 Subject: [PATCH] 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)