plots/metrics/params/experiments show: stop throwing exceptions (iter…

…ative#5984)

* params/metrics/plots/exp show: introduce onerror

* fixup

* plots/metrics/params: removing errors

* removing more errors

* removing errors

* some more removing

* new plots ux

* new ux for metrics

* new ux for params

* fixing tests

* fixups

* minor test fixes

* minor test fixes

* commands: plots/params: fix ui imports

* working on error handlng decorator, stopped at show checkpoints

* error_handler, further dvelopment

* plots just before moving loading to collect

* appending errors to results: initial passing

* fixup

* onerror: remove onerror fixture

* fixups

* plots/metrics/params/experiments: temporarily remove error warnings

* plots: extract parsing out of class method

* metrics/experiments: provide exception serializer for show-json

* params/plots/metrics: signal errors

* rebase fixups

* fix TODOs

* fixup

* skshetry's review refactor

dvc/command/experiments.py

@@ -16,6 +16,7 @@
 from dvc.exceptions import DvcException, InvalidArgumentError
 from dvc.ui import ui
 from dvc.utils.flatten import flatten
+from dvc.utils.serialize import encode_exception
 
 if TYPE_CHECKING:
     from rich.text import Text
@@ -28,6 +29,7 @@
 
 SHOW_MAX_WIDTH = 1024
 FILL_VALUE = "-"
+FILL_VALUE_ERRORED = "!"
 
 
 def _filter_name(names, label, filter_strs):
@@ -90,6 +92,7 @@ def _filter_names(
 
 def _update_names(names, items):
     for name, item in items:
+        item = item.get("data", {})
         if isinstance(item, dict):
             item = flatten(item)
             names[name].update({key: None for key in item})
@@ -100,7 +103,8 @@ def _collect_names(all_experiments, **kwargs):
     param_names = defaultdict(dict)
 
     for _, experiments in all_experiments.items():
-        for exp in experiments.values():
+        for exp_data in experiments.values():
+            exp = exp_data.get("data", {})
             _update_names(metric_names, exp.get("metrics", {}).items())
             _update_names(param_names, exp.get("params", {}).items())
     metric_names = _filter_names(
@@ -150,7 +154,8 @@ def _collect_rows(
         )
 
     new_checkpoint = True
-    for i, (rev, exp) in enumerate(experiments.items()):
+    for i, (rev, results) in enumerate(experiments.items()):
+        exp = results.get("data", {})
         if exp.get("running"):
             state = "Running"
         elif exp.get("queued"):
@@ -169,7 +174,7 @@ def _collect_rows(
         tip = exp.get("checkpoint_tip")
 
         parent_rev = exp.get("checkpoint_parent", "")
-        parent_exp = experiments.get(parent_rev, {})
+        parent_exp = experiments.get(parent_rev, {}).get("data", {})
         parent_tip = parent_exp.get("checkpoint_tip")
 
         parent = ""
@@ -202,10 +207,21 @@ def _collect_rows(
             state,
             executor,
         ]
+        fill_value = FILL_VALUE_ERRORED if results.get("error") else FILL_VALUE
         _extend_row(
-            row, metric_names, exp.get("metrics", {}).items(), precision
+            row,
+            metric_names,
+            exp.get("metrics", {}).items(),
+            precision,
+            fill_value=fill_value,
+        )
+        _extend_row(
+            row,
+            param_names,
+            exp.get("params", {}).items(),
+            precision,
+            fill_value=fill_value,
         )
-        _extend_row(row, param_names, exp.get("params", {}).items(), precision)
 
         yield row
 
@@ -268,19 +284,23 @@ def _format_time(timestamp):
     return timestamp.strftime(fmt)
 
 
-def _extend_row(row, names, items, precision):
+def _extend_row(row, names, items, precision, fill_value=FILL_VALUE):
     from rich.text import Text
 
     from dvc.compare import _format_field, with_value
 
     if not items:
-        row.extend(FILL_VALUE for keys in names.values() for _ in keys)
+        row.extend(fill_value for keys in names.values() for _ in keys)
         return
 
-    for fname, item in items:
+    for fname, data in items:
+        item = data.get("data", {})
         item = flatten(item) if isinstance(item, dict) else {fname: item}
         for name in names[fname]:
-            value = with_value(item.get(name), FILL_VALUE)
+            value = with_value(
+                item.get(name),
+                FILL_VALUE_ERRORED if data.get("error", None) else fill_value,
+            )
             # wrap field data in rich.Text, otherwise rich may
             # interpret unescaped braces from list/dict types as rich
             # markup tags
@@ -450,7 +470,7 @@ def _normalize_headers(names):
 def _format_json(item):
     if isinstance(item, (date, datetime)):
         return item.isoformat()
-    raise TypeError
+    return encode_exception(item)
 
 
 class CmdExperimentsShow(CmdBase):

dvc/command/live.py

@@ -12,12 +12,13 @@ class CmdLive(CmdBase):
     def _run(self, target, revs=None):
         metrics, plots = self.repo.live.show(target=target, revs=revs)
 
-        html_path = self.args.target + ".html"
-        self.repo.plots.write_html(html_path, plots, metrics)
+        if plots:
+            html_path = self.args.target + ".html"
+            self.repo.plots.write_html(html_path, plots, metrics)
 
-        ui.write("\nfile://", os.path.abspath(html_path), sep="")
-
-        return 0
+            ui.write("\nfile://", os.path.abspath(html_path), sep="")
+            return 0
+        return 1
 
 
 class CmdLiveShow(CmdLive):

dvc/command/metrics.py

@@ -3,8 +3,9 @@
 
 from dvc.command import completion
 from dvc.command.base import CmdBase, append_doc_link, fix_subparsers
-from dvc.exceptions import BadMetricError, DvcException
+from dvc.exceptions import DvcException
 from dvc.ui import ui
+from dvc.utils.serialize import encode_exception
 
 logger = logging.getLogger(__name__)
 
@@ -33,17 +34,10 @@ def run(self):
         if self.args.show_json:
             import json
 
-            ui.write(json.dumps(metrics))
+            ui.write(json.dumps(metrics, default=encode_exception))
         else:
             from dvc.compare import show_metrics
 
-            # 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)
-            if missing:
-                raise BadMetricError(missing)
-
             show_metrics(
                 metrics,
                 markdown=self.args.show_md,

dvc/command/plots.py

@@ -42,29 +42,35 @@ def run(self):
                 ui.write(plots[target])
                 return 0
 
-            rel: str = self.args.out or "plots.html"
-            path = (Path.cwd() / rel).resolve()
-            self.repo.plots.write_html(
-                path, plots=plots, html_template_path=self.args.html_template
-            )
-
         except DvcException:
             logger.exception("")
             return 1
 
-        assert path.is_absolute()  # as_uri throws ValueError if not absolute
-        url = path.as_uri()
-        ui.write(url)
-        if self.args.open:
-            import webbrowser
-
-            opened = webbrowser.open(rel)
-            if not opened:
-                ui.error_write(
-                    "Failed to open. Please try opening it manually."
-                )
-                return 1
+        if plots:
+            rel: str = self.args.out or "plots.html"
+            path: Path = (Path.cwd() / rel).resolve()
+            self.repo.plots.write_html(
+                path, plots=plots, html_template_path=self.args.html_template
+            )
 
+            assert (
+                path.is_absolute()
+            )  # as_uri throws ValueError if not absolute
+            url = path.as_uri()
+            ui.write(url)
+            if self.args.open:
+                import webbrowser
+
+                opened = webbrowser.open(rel)
+                if not opened:
+                    ui.error_write(
+                        "Failed to open. Please try opening it manually."
+                    )
+                    return 1
+        else:
+            ui.error_write(
+                "No plots were loaded, visualization file will not be created."
+            )
         return 0
 
 

dvc/compare.py

@@ -281,8 +281,9 @@ def metrics_table(
     td = TabularData(["Revision", "Path"], fill_value="-")
 
     for branch, val in metrics.items():
-        for fname, metric in val.items():
+        for fname, metric in val.get("data", {}).items():
             row_data: Dict[str, str] = {"Revision": branch, "Path": fname}
+            metric = metric.get("data", {})
             flattened = (
                 flatten(format_dict(metric))
                 if isinstance(metric, dict)

dvc/dependency/param.py

@@ -98,7 +98,7 @@ def _read(self):
                 f"Unable to read parameters from '{self}'"
             ) from exc
 
-    def read_params_d(self):
+    def read_params_d(self, **kwargs):
         config = self._read()
 
         ret = {}

dvc/exceptions.py

@@ -1,5 +1,4 @@
 """Exceptions raised by the dvc."""
-from typing import List
 
 
 class DvcException(Exception):
@@ -173,36 +172,6 @@ def __init__(self, paths):
         )
 
 
-class MetricsError(DvcException):
-    pass
-
-
-class NoMetricsParsedError(MetricsError):
-    def __init__(self, command):
-        super().__init__(
-            f"Could not parse {command} files. Use `-v` option to see more "
-            "details."
-        )
-
-
-class NoMetricsFoundError(MetricsError):
-    def __init__(self, command, run_options):
-        super().__init__(
-            f"No {command} files in this repository. "
-            f"Use `{run_options}` options for "
-            f"`dvc run` to mark stage outputs as {command}."
-        )
-
-
-class MetricDoesNotExistError(MetricsError):
-    def __init__(self, targets: List[str]):
-        if len(targets) == 1:
-            msg = "'{}' does not exist."
-        else:
-            msg = "'{}' do not exist."
-        super().__init__(msg.format(", ".join(targets)))
-
-
 class RecursiveAddingWhileUsingFilename(DvcException):
     def __init__(self):
         super().__init__(

dvc/repo/collect.py

@@ -46,15 +46,12 @@ def _collect_paths(
             target_infos.extend(repo.dvcignore.walk_files(fs, path_info))
 
         if not fs.exists(path_info):
-            if not recursive:
-                if rev == "workspace" or rev == "":
-                    logger.warning(
-                        "'%s' was not found in current workspace.", path_info
-                    )
-                else:
-                    logger.warning(
-                        "'%s' was not found at: '%s'.", path_info, rev
-                    )
+            if rev == "workspace" or rev == "":
+                logger.warning(
+                    "'%s' was not found in current workspace.", path_info
+                )
+            else:
+                logger.warning("'%s' was not found at: '%s'.", path_info, rev)
             continue
         target_infos.append(path_info)
     return target_infos

dvc/repo/experiments/diff.py

@@ -35,8 +35,8 @@ def diff(repo, *args, a_rev=None, b_rev=None, param_deps=False, **kwargs):
 
     return {
         key: _diff(
-            format_dict(old[key]),
-            format_dict(new[key]),
+            format_dict(old.get("data", {}).get(key, {})),
+            format_dict(new.get("data", {}).get(key, {})),
             with_unchanged=with_unchanged,
         )
         for key in ["metrics", "params"]