Skip to content

Commit

Permalink
plots: return errors in json format (#9146)
Browse files Browse the repository at this point in the history
* plots: return errors in json format

* plots --json: move plots data into data key

* rename  revision key to rev

* move imports to top-level

* add tests
  • Loading branch information
skshetry authored Mar 27, 2023
1 parent d41512c commit 43195a8
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 49 deletions.
43 changes: 37 additions & 6 deletions dvc/commands/plots.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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))
Expand Down
35 changes: 29 additions & 6 deletions dvc/render/match.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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
4 changes: 2 additions & 2 deletions dvc/repo/plots/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 5 additions & 2 deletions tests/func/plots/test_show.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
83 changes: 59 additions & 24 deletions tests/integration/plots/test_plots.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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():
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -326,24 +344,24 @@ 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,
"workspace",
"../image.png",
image_v2,
html_path,
json_result,
json_data,
)
verify_image(
path,
"HEAD",
"../image.png",
image_v1,
html_path,
json_result,
json_data,
)


Expand All @@ -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):
Expand Down
13 changes: 9 additions & 4 deletions tests/unit/command/test_plots.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
)
Expand Down Expand Up @@ -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)

Expand Down
Loading

0 comments on commit 43195a8

Please sign in to comment.