cleanup metrics help, fix type and xpath handling

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)

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)
 

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)
 

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))

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()

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
 

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)