diff --git a/dvc/commands/plots.py b/dvc/commands/plots.py index b999759482..9382ce9c23 100644 --- a/dvc/commands/plots.py +++ b/dvc/commands/plots.py @@ -1,8 +1,9 @@ import argparse import logging import os +from typing import TYPE_CHECKING, Dict, List -from funcy import first +from funcy import compact, first from dvc.cli import completion from dvc.cli.command import CmdBase @@ -11,14 +12,43 @@ from dvc.ui import ui from dvc.utils import format_link +if TYPE_CHECKING: + from dvc.render.match import RendererWithErrors + + logger = logging.getLogger(__name__) -def _show_json(renderers, split=False): +def _show_json(renderers_with_errors: List["RendererWithErrors"], split=False): from dvc.render.convert import to_json + from dvc.utils.serialize import encode_exception + + errors: List[Dict] = [] + data = {} + + for renderer, src_errors, def_errors in renderers_with_errors: + name = renderer.name + data[name] = to_json(renderer, split) + errors.extend( + { + "name": name, + "rev": rev, + "source": source, + **encode_exception(e), + } + for rev, per_rev_src_errors in src_errors.items() + for source, e in per_rev_src_errors.items() + ) + errors.extend( + { + "name": name, + "rev": rev, + **encode_exception(e), + } + for rev, e in def_errors.items() + ) - result = {renderer.name: to_json(renderer, split) for renderer in renderers} - ui.write_json(result, highlight=False) + ui.write_json(compact({"errors": errors, "data": data}), highlight=False) def _adjust_vega_renderers(renderers): @@ -122,15 +152,16 @@ def run(self) -> int: # noqa: C901, PLR0911, PLR0912 renderers_out = out if self.args.json else os.path.join(out, "static") - renderers = match_defs_renderers( + renderers_with_errors = match_defs_renderers( data=plots_data, out=renderers_out, templates_dir=self.repo.plots.templates_dir, ) if self.args.json: - _show_json(renderers, self.args.split) + _show_json(renderers_with_errors, self.args.split) return 0 + renderers = [r.renderer for r in renderers_with_errors] _adjust_vega_renderers(renderers) if self.args.show_vega: renderer = first(filter(lambda r: r.TYPE == "vega", renderers)) diff --git a/dvc/render/match.py b/dvc/render/match.py index 196f47f65e..3f36e9adc0 100644 --- a/dvc/render/match.py +++ b/dvc/render/match.py @@ -1,10 +1,10 @@ import os from collections import defaultdict -from typing import TYPE_CHECKING, Dict, List, Optional +from typing import TYPE_CHECKING, DefaultDict, Dict, List, NamedTuple, Optional import dpath import dpath.options -from funcy import last +from funcy import get_in, last from dvc.repo.plots import _normpath, infer_data_sources from dvc.utils.plots import get_plot_id @@ -13,6 +13,8 @@ if TYPE_CHECKING: from dvc.types import StrPath + from dvc_render.base import Renderer + dpath.options.ALLOW_EMPTY_STRING_KEYS = True @@ -61,21 +63,31 @@ def get_definition_data(self, target_files, rev): return result -def match_defs_renderers( +class RendererWithErrors(NamedTuple): + renderer: "Renderer" + source_errors: Dict[str, Dict[str, Exception]] + definition_errors: Dict[str, Exception] + + +def match_defs_renderers( # noqa: C901, PLR0912 data, out=None, templates_dir: Optional["StrPath"] = None, -): +) -> List[RendererWithErrors]: from dvc_render import ImageRenderer, VegaRenderer plots_data = PlotsData(data) renderers = [] renderer_cls = None + for plot_id, group in plots_data.group_definitions().items(): plot_datapoints: List[Dict] = [] props = _squash_plots_properties(group) final_props: Dict = {} + def_errors: Dict[str, Exception] = {} + src_errors: DefaultDict[str, Dict[str, Exception]] = defaultdict(dict) + if out is not None: props["out"] = out if templates_dir is not None: @@ -94,13 +106,24 @@ def match_defs_renderers( converter = _get_converter(renderer_cls, inner_id, props, definitions_data) - dps, rev_props = converter.flat_datapoints(rev) + for src in plot_sources: + if error := get_in(data, [rev, "sources", "data", src, "error"]): + src_errors[rev][src] = error + + try: + dps, rev_props = converter.flat_datapoints(rev) + except Exception as e: # noqa: BLE001, pylint: disable=broad-except + def_errors[rev] = e + continue + if not final_props and rev_props: final_props = rev_props plot_datapoints.extend(dps) if "title" not in final_props: final_props["title"] = renderer_id + if renderer_cls is not None: - renderers.append(renderer_cls(plot_datapoints, renderer_id, **final_props)) + renderer = renderer_cls(plot_datapoints, renderer_id, **final_props) + renderers.append(RendererWithErrors(renderer, dict(src_errors), def_errors)) return renderers diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py index fd7a12303e..281a8e8b28 100644 --- a/dvc/repo/plots/__init__.py +++ b/dvc/repo/plots/__init__.py @@ -20,7 +20,7 @@ import dpath import dpath.options -from funcy import distinct, first, project, reraise +from funcy import first, ldistinct, project, reraise from dvc.exceptions import DvcException from dvc.utils import error_handler, errored_revisions, onerror_collect @@ -356,7 +356,7 @@ def infer_data_sources(plot_id, config=None): if isinstance(x, dict): sources.append(first(x.keys())) - return distinct(source for source in sources) + return ldistinct(source for source in sources) def _matches(targets, config_file, plot_id): diff --git a/tests/func/plots/test_show.py b/tests/func/plots/test_show.py index 0d49745abc..0a31c7fcf7 100644 --- a/tests/func/plots/test_show.py +++ b/tests/func/plots/test_show.py @@ -418,5 +418,8 @@ def test_show_plots_defined_with_native_os_path(tmp_dir, dvc, scm, capsys): assert main(["plots", "show", "--json"]) == 0 out, _ = capsys.readouterr() json_out = json.loads(out) - assert json_out[f"dvc.yaml::{top_level_plot}"] - assert json_out[stage_plot] + assert "errors" not in json_out + + json_data = json_out["data"] + assert json_data[f"dvc.yaml::{top_level_plot}"] + assert json_data[stage_plot] diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index c8db3923ec..4119f46918 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -182,7 +182,13 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ html_path, json_result, split_json_result = call(capsys) html_result = extract_vega_specs(html_path, ["linear.json", "confusion.json"]) - assert json_result["linear.json"][0]["content"]["data"][ + assert "errors" not in json_result + assert "errors" not in split_json_result + + json_data = json_result["data"] + split_json_data = split_json_result["data"] + + assert json_data["linear.json"][0]["content"]["data"][ "values" ] == _update_datapoints( linear_v1, @@ -200,7 +206,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ REVISION_FIELD: "workspace", }, ) - assert json_result["confusion.json"][0]["content"]["data"][ + assert json_data["confusion.json"][0]["content"]["data"][ "values" ] == _update_datapoints( confusion_v1, @@ -218,34 +224,40 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ REVISION_FIELD: "workspace", }, ) - verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_result) + verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_data) for plot in ["linear.json", "confusion.json"]: verify_vega( "workspace", html_result[plot], - json_result[plot], - split_json_result[plot], + json_data[plot], + split_json_data[plot], ) - verify_vega_props("confusion.json", json_result, **confusion_props) + verify_vega_props("confusion.json", json_data, **confusion_props) image_v2, linear_v2, confusion_v2, confusion_props = next(repo_state) html_path, json_result, split_json_result = call(capsys, subcommand="diff") html_result = extract_vega_specs(html_path, ["linear.json", "confusion.json"]) - verify_image(tmp_dir, "workspace", "image.png", image_v2, html_path, json_result) - verify_image(tmp_dir, "HEAD", "image.png", image_v1, html_path, json_result) + assert "errors" not in json_result + assert "errors" not in split_json_result + + json_data = json_result["data"] + split_json_data = split_json_result["data"] + + verify_image(tmp_dir, "workspace", "image.png", image_v2, html_path, json_data) + verify_image(tmp_dir, "HEAD", "image.png", image_v1, html_path, json_data) for plot in ["linear.json", "confusion.json"]: verify_vega( ["HEAD", "workspace"], html_result[plot], - json_result[plot], - split_json_result[plot], + json_data[plot], + split_json_data[plot], ) - verify_vega_props("confusion.json", json_result, **confusion_props) + verify_vega_props("confusion.json", json_data, **confusion_props) path = tmp_dir / "subdir" path.mkdir() with path.chdir(): @@ -254,7 +266,13 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ html_path, ["../linear.json", "../confusion.json"], ) - assert json_result["../linear.json"][0]["content"]["data"][ + + assert "errors" not in json_result + assert "errors" not in split_json_result + + json_data = json_result["data"] + split_json_data = split_json_result["data"] + assert json_data["../linear.json"][0]["content"]["data"][ "values" ] == _update_datapoints( linear_v2, @@ -286,7 +304,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ REVISION_FIELD: "HEAD", }, ) - assert json_result["../confusion.json"][0]["content"]["data"][ + assert json_data["../confusion.json"][0]["content"]["data"][ "values" ] == _update_datapoints( confusion_v2, @@ -326,8 +344,8 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ verify_vega( ["HEAD", "workspace"], html_result[plot], - json_result[plot], - split_json_result[plot], + json_data[plot], + split_json_data[plot], ) verify_image( path, @@ -335,7 +353,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ "../image.png", image_v2, html_path, - json_result, + json_data, ) verify_image( path, @@ -343,7 +361,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ "../image.png", image_v1, html_path, - json_result, + json_data, ) @@ -361,13 +379,30 @@ def test_repo_with_removed_plots(tmp_dir, capsys, repo_with_plots): for s in {"show", "diff"}: _, json_result, split_json_result = call(capsys, subcommand=s) - for p in { - "linear.json", - "confusion.json", - "image.png", - }: - assert json_result[p] == [] - assert split_json_result[p] == [] + errors = [ + { + "name": p, + "source": p, + "rev": "workspace", + "type": "FileNotFoundError", + "msg": "", + } + for p in [ + "linear.json", + "confusion.json", + "image.png", + ] + ] + expected_result = { + "errors": errors, + "data": { + "image.png": [], + "confusion.json": [], + "linear.json": [], + }, + } + assert json_result == expected_result + assert split_json_result == expected_result def test_config_output_dir(tmp_dir, dvc, capsys): diff --git a/tests/unit/command/test_plots.py b/tests/unit/command/test_plots.py index bb2f67ce4d..1c9e5d4610 100644 --- a/tests/unit/command/test_plots.py +++ b/tests/unit/command/test_plots.py @@ -7,6 +7,7 @@ from dvc.cli import parse_args from dvc.commands.plots import CmdPlotsDiff, CmdPlotsShow, CmdPlotsTemplates +from dvc.render.match import RendererWithErrors @pytest.fixture @@ -268,8 +269,11 @@ def test_should_call_render(tmp_dir, mocker, capsys, plots_data, output): output = output or "dvc_plots" index_path = tmp_dir / output / "index.html" - renderers = mocker.MagicMock() - mocker.patch("dvc.render.match.match_defs_renderers", return_value=renderers) + renderer = mocker.MagicMock() + mocker.patch( + "dvc.render.match.match_defs_renderers", + return_value=[RendererWithErrors(renderer, {}, {})], + ) render_mock = mocker.patch("dvc_render.render_html", return_value=index_path) assert cmd.run() == 0 @@ -278,7 +282,7 @@ def test_should_call_render(tmp_dir, mocker, capsys, plots_data, output): assert index_path.as_uri() in out render_mock.assert_called_once_with( - renderers=renderers, + renderers=[renderer], output_file=Path(tmp_dir / output / "index.html"), html_template=None, ) @@ -354,12 +358,13 @@ def test_show_json(split, mocker, capsys): import dvc.commands.plots renderer = mocker.MagicMock() + renderer_obj = RendererWithErrors(renderer, {}, {}) renderer.name = "rname" to_json_mock = mocker.patch( "dvc.render.convert.to_json", return_value={"renderer": "json"} ) - dvc.commands.plots._show_json([renderer], split) + dvc.commands.plots._show_json([renderer_obj], split) to_json_mock.assert_called_once_with(renderer, split) diff --git a/tests/unit/render/test_match.py b/tests/unit/render/test_match.py index 02316559ab..822d37283a 100644 --- a/tests/unit/render/test_match.py +++ b/tests/unit/render/test_match.py @@ -1,4 +1,7 @@ +from funcy import set_in + from dvc.render import VERSION_FIELD +from dvc.render.converter.vega import VegaConverter from dvc.render.match import PlotsData, _squash_plots_properties, match_defs_renderers @@ -35,7 +38,7 @@ def test_group_definitions(): } -def test_match_renderers(mocker): +def test_match_renderers(M): data = { "v1": { "definitions": { @@ -77,9 +80,9 @@ def test_match_renderers(mocker): }, } - renderers = match_defs_renderers(data) - assert len(renderers) == 1 - assert renderers[0].datapoints == [ + (renderer_with_errors,) = match_defs_renderers(data) + renderer = renderer_with_errors[0] + assert renderer.datapoints == [ { VERSION_FIELD: { "revision": "v1", @@ -99,13 +102,31 @@ def test_match_renderers(mocker): "y": 2, }, ] - assert renderers[0].properties == { + assert renderer.properties == { "title": "config_file_1::plot_id_1", "x": "x", "y": "y", "x_label": "x", "y_label": "y", } + assert renderer_with_errors.source_errors == { + "revision_with_no_data": {"file.json": M.instance_of(FileNotFoundError)} + } + assert not renderer_with_errors.definition_errors + + +def test_flat_datapoints_errors_are_caught(M, mocker): + d = {} + d = set_in( + d, + ["v1", "definitions", "data", "dvc.yaml", "data", "plot_id_1"], + {"x": "x", "y": {"file.json": "y"}}, + ) + d = set_in(d, ["v1", "sources", "data", "file.json", "data"], [{"x": 1, "y": 1}]) + mocker.patch.object(VegaConverter, "flat_datapoints", side_effect=ValueError) + (renderer_with_errors,) = match_defs_renderers(d) + assert not renderer_with_errors.source_errors + assert renderer_with_errors.definition_errors == {"v1": M.instance_of(ValueError)} def test_squash_plots_properties():