Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plots/metrics/params/experiments show: stop throwing exceptions #5984

Merged
merged 29 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
66055bc
params/metrics/plots/exp show: introduce onerror
pared May 25, 2021
e38c0b0
fixup
pared May 26, 2021
628f013
plots/metrics/params: removing errors
pared May 26, 2021
64c36e2
removing more errors
pared May 26, 2021
d5ae83a
removing errors
pared May 26, 2021
f6c7bff
some more removing
pared May 26, 2021
267bd9c
new plots ux
pared May 26, 2021
5d0e1a8
new ux for metrics
pared May 27, 2021
8a45018
new ux for params
pared May 27, 2021
a0427a3
fixing tests
pared May 28, 2021
115d5c3
fixups
pared May 28, 2021
75bf1bf
minor test fixes
pared Jun 9, 2021
b4ee1c4
minor test fixes
pared Jun 9, 2021
33ec341
commands: plots/params: fix ui imports
pared Jun 22, 2021
7dd7f8c
working on error handlng decorator, stopped at show checkpoints
pared Jun 25, 2021
dee1596
error_handler, further dvelopment
pared Jun 30, 2021
dcbc2f2
plots just before moving loading to collect
pared Jul 6, 2021
ce878d0
appending errors to results: initial passing
pared Jul 7, 2021
38c0aaa
fixup
pared Jul 7, 2021
e9e82bb
onerror: remove onerror fixture
pared Jul 8, 2021
88624b8
fixups
pared Jul 8, 2021
934c35a
plots/metrics/params/experiments: temporarily remove error warnings
pared Jul 8, 2021
5ac9a36
plots: extract parsing out of class method
pared Jul 8, 2021
39b3d77
metrics/experiments: provide exception serializer for show-json
pared Jul 9, 2021
0a8e9bc
params/plots/metrics: signal errors
pared Jul 12, 2021
936624e
rebase fixups
pared Jul 12, 2021
325c160
fix TODOs
pared Jul 13, 2021
cd4c95b
fixup
pared Jul 13, 2021
fe78054
skshetry's review refactor
pared Jul 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 30 additions & 10 deletions dvc/command/experiments.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,6 +29,7 @@

SHOW_MAX_WIDTH = 1024
FILL_VALUE = "-"
FILL_VALUE_ERRORED = "!"


def _filter_name(names, label, filter_strs):
Expand Down Expand Up @@ -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})
Expand All @@ -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(
Expand Down Expand Up @@ -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"):
Expand All @@ -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 = ""
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
11 changes: 6 additions & 5 deletions dvc/command/live.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
12 changes: 3 additions & 9 deletions dvc/command/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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)

Comment on lines -40 to -46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we don't raise any error now?

Copy link
Contributor Author

@pared pared Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had 2 reasons for removing that:

  1. I was unable to retrigger this behavior.
  2. New approach assumes we do not throw exceptions but rather gather them. Lack of metrics will be represented as empty files content.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should/can we at least provide a warning or something that some metrics are missing?

show_metrics(
metrics,
markdown=self.args.show_md,
Expand Down
42 changes: 24 additions & 18 deletions dvc/command/plots.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
3 changes: 2 additions & 1 deletion dvc/compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion dvc/dependency/param.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand Down
31 changes: 0 additions & 31 deletions dvc/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Exceptions raised by the dvc."""
from typing import List


class DvcException(Exception):
Expand Down Expand Up @@ -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__(
Expand Down
15 changes: 6 additions & 9 deletions dvc/repo/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions dvc/repo/experiments/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down