From 1e99e9fbe71dd86b3223df8ae0604d18eaf18659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Redzy=C5=84ski?= Date: Tue, 3 Nov 2020 14:46:11 +0100 Subject: [PATCH] plots/metrics: unify no metrics/plots messaging --- dvc/command/plots.py | 2 +- dvc/exceptions.py | 17 ++++++-- dvc/repo/metrics/show.py | 13 ++---- dvc/repo/plots/__init__.py | 24 +++++++---- dvc/repo/plots/data.py | 42 +++++++++----------- tests/func/metrics/plots/test_show.py | 7 ---- tests/func/metrics/test_common.py | 57 ++++++++++++++++----------- 7 files changed, 87 insertions(+), 75 deletions(-) diff --git a/dvc/command/plots.py b/dvc/command/plots.py index 04e5b2918d..775214c036 100644 --- a/dvc/command/plots.py +++ b/dvc/command/plots.py @@ -73,7 +73,7 @@ def run(self): ) except DvcException: - logger.exception("") + logger.exception("failed to show plots") return 1 return 0 diff --git a/dvc/exceptions.py b/dvc/exceptions.py index 17b4a87eda..1a4a44f99d 100644 --- a/dvc/exceptions.py +++ b/dvc/exceptions.py @@ -174,11 +174,20 @@ class NoMetricsError(DvcException): pass -class NoPlotsError(DvcException): - def __init__(self): +class NoMetricsParsedError(NoMetricsError): + def __init__(self, command): + super().__init__( + f"Could not parse {command} files. Use `-v` option to see more " + "details." + ) + + +class NoMetricsFoundError(NoMetricsError): + def __init__(self, command, run_options): super().__init__( - "no plots in this repository. Use `--plots/--plots-no-cache` " - "options for `dvc run` to mark stage outputs as plots." + f"No {command} files in this repository. " + f"Use `{run_options}` options for " + f"`dvc run` to mark stage outputs as {command}." ) diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 28e1816c07..6a08224c45 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -1,6 +1,6 @@ import logging -from dvc.exceptions import NoMetricsError +from dvc.exceptions import NoMetricsFoundError, NoMetricsParsedError from dvc.repo import locked from dvc.repo.collect import collect from dvc.scm.base import SCMError @@ -104,16 +104,9 @@ def show( if not res: if metrics_found: - msg = ( - "Could not parse metrics files. Use `-v` option to see more " - "details." - ) + raise NoMetricsParsedError("metrics") else: - msg = ( - "no metrics files in this repository. Use `-m/-M` options for " - "`dvc run` to mark stage outputs as metrics." - ) - raise NoMetricsError(msg) + raise NoMetricsFoundError("metrics", "-m/-M") # Hide workspace metrics if they are the same as in the active branch try: diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py index 233e60672d..e4ad6fa390 100644 --- a/dvc/repo/plots/__init__.py +++ b/dvc/repo/plots/__init__.py @@ -2,7 +2,11 @@ from funcy import cached_property, first, project -from dvc.exceptions import DvcException, NoPlotsError +from dvc.exceptions import ( + DvcException, + NoMetricsFoundError, + NoMetricsParsedError, +) from dvc.repo.collect import collect from dvc.repo.plots.data import PlotParsingError from dvc.schema import PLOT_PROPS @@ -65,7 +69,13 @@ def collect(self, targets=None, revs=None): return data @staticmethod - def render(plots, templates=None): + def render(data, revs=None, props=None, templates=None): + """Renders plots""" + props = props or {} + + # Merge data by plot file and apply overriding props + plots = _prepare_plots(data, revs, props) + result = {} for datafile, desc in plots.items(): try: @@ -81,7 +91,8 @@ def render(plots, templates=None): ) if not any(result.values()): - raise DvcException("Failed to parse any plot.") + raise NoMetricsParsedError("plots") + return result def show(self, targets=None, revs=None, props=None, templates=None): @@ -98,14 +109,11 @@ def show(self, targets=None, revs=None, props=None, templates=None): # No data at all is a special error with a special message if not data: - raise NoPlotsError() + raise NoMetricsFoundError("plots", "--plots/--plots-no-cache") if templates is None: templates = self.templates - - plots = _prepare_plots(data, revs, props or {}) - - return self.render(plots, templates) + return self.render(data, revs, props, templates) def diff(self, *args, **kwargs): from .diff import diff diff --git a/dvc/repo/plots/data.py b/dvc/repo/plots/data.py index 2eea1d95b2..bff7aeb3d4 100644 --- a/dvc/repo/plots/data.py +++ b/dvc/repo/plots/data.py @@ -167,7 +167,8 @@ def _processors(self): def to_datapoints(self, **kwargs): with reraise( - ParseError, PlotParsingError(self.filename, self.revision) + [ParseError, csv.Error], + PlotParsingError(self.filename, self.revision), ): data = self.raw(**kwargs) @@ -195,31 +196,26 @@ def __init__(self, filename, revision, content, delimiter=","): self.delimiter = delimiter def raw(self, header=True, **kwargs): # pylint: disable=arguments-differ - with reraise( - csv.Error, ParseError(self.filename, f"revision: {self.revision}") - ): - first_row = first(csv.reader(io.StringIO(self.content))) + first_row = first(csv.reader(io.StringIO(self.content))) - if header: - reader = csv.DictReader( - io.StringIO(self.content), delimiter=self.delimiter, - ) - else: - reader = csv.DictReader( - io.StringIO(self.content), - delimiter=self.delimiter, - fieldnames=[str(i) for i in range(len(first_row))], - ) + if header: + reader = csv.DictReader( + io.StringIO(self.content), delimiter=self.delimiter, + ) + else: + reader = csv.DictReader( + io.StringIO(self.content), + delimiter=self.delimiter, + fieldnames=[str(i) for i in range(len(first_row))], + ) - fieldnames = reader.fieldnames - data = list(reader) + fieldnames = reader.fieldnames + data = list(reader) - return [ - OrderedDict( - [(field, data_point[field]) for field in fieldnames] - ) - for data_point in data - ] + return [ + OrderedDict([(field, data_point[field]) for field in fieldnames]) + for data_point in data + ] class YAMLPlotData(PlotData): diff --git a/tests/func/metrics/plots/test_show.py b/tests/func/metrics/plots/test_show.py index 0e4f05e532..f4a29986c2 100644 --- a/tests/func/metrics/plots/test_show.py +++ b/tests/func/metrics/plots/test_show.py @@ -398,13 +398,6 @@ def _replace(path, src, dst): path.write_text(path.read_text().replace(src, dst)) -def test_no_plots(tmp_dir, dvc): - from dvc.exceptions import NoPlotsError - - with pytest.raises(NoPlotsError): - dvc.plots.show() - - def test_should_raise_on_no_template(tmp_dir, dvc, run_copy_metrics): metric = [{"val": 2}, {"val": 3}] _write_json(tmp_dir, metric, "metric_t.json") diff --git a/tests/func/metrics/test_common.py b/tests/func/metrics/test_common.py index 9071fdd103..d0ebc37156 100644 --- a/tests/func/metrics/test_common.py +++ b/tests/func/metrics/test_common.py @@ -2,44 +2,57 @@ import pytest +from dvc.main import main from tests.func.metrics.utils import _write_json -def metrics_diff(dvc, filename, revision): - dvc.metrics.diff(targets=[filename], a_rev=revision) - - -def plots_diff(dvc, filename, revision): - dvc.plots.diff(targets=[filename], revs=[revision]) - - @pytest.mark.parametrize( - "diff_fun, metric_value", - ((metrics_diff, {"m": 1}), (plots_diff, [{"m": 1}, {"m": 2}])), + "command, metric_value", + (("metrics", {"m": 1}), ("plots", [{"m": 1}, {"m": 2}])), ) def test_diff_no_file_on_target_rev( - tmp_dir, scm, dvc, caplog, diff_fun, metric_value + tmp_dir, scm, dvc, caplog, command, metric_value ): with tmp_dir.branch("new_branch", new=True): _write_json(tmp_dir, metric_value, "metric.json") with caplog.at_level(logging.WARNING, "dvc"): - diff_fun(dvc, "metric.json", "master") + assert ( + main([command, "diff", "master", "--targets", "metric.json"]) + == 0 + ) assert "'metric.json' was not found at: 'master'." in caplog.text @pytest.mark.parametrize( - "fun, malformed_metric", - ( - (lambda repo: repo.metrics, '{"m": 1'), - (lambda repo: repo.plots, '[{"m": 1}, {"m": 2}'), - ), + "command, malformed_metric", + (("metrics", '{"m": 1'), ("plots", '[{"m": 1}, {"m": 2}'),), ) -def test_show_malformed_metric(tmp_dir, scm, dvc, fun, malformed_metric): +def test_show_malformed_metric( + tmp_dir, scm, dvc, caplog, command, malformed_metric +): tmp_dir.gen("metric.json", malformed_metric) - _write_json( - tmp_dir, {"val": 1, "val2": [{"p": 1}, {"p": 2}]}, "metric_proper.json" - ) - fun(dvc).show(targets=["metric.json", "metric_proper.json"]) + with caplog.at_level(logging.ERROR, "dvc"): + assert main([command, "show", "metric.json"]) == 1 + + assert ( + f"Could not parse {command} files. " + "Use `-v` option to see more details." + ) in caplog.text + + +@pytest.mark.parametrize( + "command, run_options", + (("metrics", "-m/-M"), ("plots", "--plots/--plots-no-cache"),), +) +def test_show_no_metrics_files(tmp_dir, dvc, caplog, command, run_options): + with caplog.at_level(logging.ERROR, "dvc"): + assert main([command, "show"]) == 1 + + assert ( + f"No {command} files in this repository. " + f"Use `{run_options}` options for " + f"`dvc run` to mark stage outputs as {command}." + ) in caplog.text