From 0015ef91f21667711d02f662a560309248e376c5 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 11 Sep 2023 20:19:51 +1000 Subject: [PATCH 01/14] standardise plot data output --- dvc/commands/plots.py | 44 ----- dvc/render/__init__.py | 2 +- dvc/render/convert.py | 4 +- dvc/render/converter/vega.py | 10 +- tests/integration/plots/test_plots.py | 75 ++++---- tests/unit/render/test_convert.py | 18 +- tests/unit/render/test_match.py | 17 +- tests/unit/render/test_vega_converter.py | 209 +++++++++-------------- 8 files changed, 140 insertions(+), 239 deletions(-) diff --git a/dvc/commands/plots.py b/dvc/commands/plots.py index 498f78ad2c..a53be3f35e 100644 --- a/dvc/commands/plots.py +++ b/dvc/commands/plots.py @@ -59,49 +59,6 @@ def _show_json( ui.write_json(compact({"errors": all_errors, "data": data}), highlight=False) -def _adjust_vega_renderers(renderers): - from dvc.render import REVISION_FIELD, VERSION_FIELD - from dvc_render import VegaRenderer - - for r in renderers: - if isinstance(r, VegaRenderer): - if _data_versions_count(r) > 1: - summary = _summarize_version_infos(r) - for dp in r.datapoints: - vi = dp.pop(VERSION_FIELD, {}) - keys = list(vi.keys()) - for key in keys: - if not (len(summary.get(key, set())) > 1): - vi.pop(key) - if vi: - dp["rev"] = "::".join(vi.values()) - else: - for dp in r.datapoints: - dp[REVISION_FIELD] = dp[VERSION_FIELD]["revision"] - dp.pop(VERSION_FIELD, {}) - - -def _summarize_version_infos(renderer): - from collections import defaultdict - - from dvc.render import VERSION_FIELD - - result = defaultdict(set) - - for dp in renderer.datapoints: - for key, value in dp.get(VERSION_FIELD, {}).items(): - result[key].add(value) - return dict(result) - - -def _data_versions_count(renderer): - from itertools import product - - summary = _summarize_version_infos(renderer) - x = product(summary.get("filename", {None}), summary.get("field", {None})) - return len(set(x)) - - class CmdPlots(CmdBase): def _func(self, *args, **kwargs): raise NotImplementedError @@ -175,7 +132,6 @@ def run(self) -> int: # noqa: C901, PLR0911, PLR0912 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)) if renderer: diff --git a/dvc/render/__init__.py b/dvc/render/__init__.py index bedc1414cf..b5c6c747a8 100644 --- a/dvc/render/__init__.py +++ b/dvc/render/__init__.py @@ -1,7 +1,7 @@ INDEX_FIELD = "step" REVISION_FIELD = "rev" FILENAME_FIELD = "filename" -VERSION_FIELD = "dvc_data_version_info" +FIELD_FIELD = "field" REVISIONS_KEY = "revisions" TYPE_KEY = "type" SRC_FIELD = "src" diff --git a/dvc/render/convert.py b/dvc/render/convert.py index 1d576fc3ab..a61de29162 100644 --- a/dvc/render/convert.py +++ b/dvc/render/convert.py @@ -1,7 +1,7 @@ from collections import defaultdict from typing import Dict, List, Union -from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY, VERSION_FIELD +from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY from dvc.render.converter.image import ImageConverter from dvc.render.converter.vega import VegaConverter @@ -22,7 +22,7 @@ def _get_converter( def _group_by_rev(datapoints): grouped = defaultdict(list) for datapoint in datapoints: - rev = datapoint.get(VERSION_FIELD, {}).get("revision") + rev = datapoint.get(REVISION_FIELD) grouped[rev].append(datapoint) return dict(grouped) diff --git a/dvc/render/converter/vega.py b/dvc/render/converter/vega.py index 6b45be7131..4ede2d3d1b 100644 --- a/dvc/render/converter/vega.py +++ b/dvc/render/converter/vega.py @@ -4,7 +4,7 @@ from funcy import first, last from dvc.exceptions import DvcException -from dvc.render import FILENAME_FIELD, INDEX_FIELD, VERSION_FIELD +from dvc.render import FIELD_FIELD, FILENAME_FIELD, INDEX_FIELD, REVISION_FIELD from . import Converter @@ -270,11 +270,9 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 _update_all( datapoints, update_dict={ - VERSION_FIELD: { - "revision": revision, - FILENAME_FIELD: y_file_short, - "field": y_field, - } + REVISION_FIELD: revision, + FILENAME_FIELD: y_file_short, + FIELD_FIELD: y_field, }, ) diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 8a22918b92..6a46534047 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -11,7 +11,7 @@ from funcy import first from dvc.cli import main -from dvc.render import REVISION_FIELD, VERSION_FIELD +from dvc.render import REVISION_FIELD JSON_OUT = "vis_data" @@ -193,17 +193,17 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( linear_v1, { - VERSION_FIELD: { - "revision": "workspace", - "filename": "linear.json", - "field": "y", - }, + "rev": "workspace", + "filename": "linear.json", + "field": "y", }, ) assert html_result["linear.json"]["data"]["values"] == _update_datapoints( linear_v1, { REVISION_FIELD: "workspace", + "filename": "linear.json", + "field": "y", }, ) assert json_data["confusion.json"][0]["content"]["data"][ @@ -211,17 +211,17 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( confusion_v1, { - VERSION_FIELD: { - "revision": "workspace", - "filename": "confusion.json", - "field": "actual", - }, + "rev": "workspace", + "filename": "confusion.json", + "field": "actual", }, ) assert html_result["confusion.json"]["data"]["values"] == _update_datapoints( confusion_v1, { - REVISION_FIELD: "workspace", + "rev": "workspace", + "filename": "confusion.json", + "field": "actual", }, ) verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_data) @@ -277,31 +277,31 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( linear_v2, { - VERSION_FIELD: { - "revision": "workspace", - "filename": "../linear.json", - "field": "y", - }, + "rev": "workspace", + "filename": "../linear.json", + "field": "y", }, ) + _update_datapoints( linear_v1, { - VERSION_FIELD: { - "revision": "HEAD", - "filename": "../linear.json", - "field": "y", - }, + "rev": "HEAD", + "filename": "../linear.json", + "field": "y", }, ) assert html_result["../linear.json"]["data"]["values"] == _update_datapoints( linear_v2, { REVISION_FIELD: "workspace", + "filename": "../linear.json", + "field": "y", }, ) + _update_datapoints( linear_v1, { REVISION_FIELD: "HEAD", + "filename": "../linear.json", + "field": "y", }, ) assert json_data["../confusion.json"][0]["content"]["data"][ @@ -309,31 +309,31 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( confusion_v2, { - VERSION_FIELD: { - "revision": "workspace", - "filename": "../confusion.json", - "field": "actual", - }, + "rev": "workspace", + "filename": "../confusion.json", + "field": "actual", }, ) + _update_datapoints( confusion_v1, { - VERSION_FIELD: { - "revision": "HEAD", - "filename": "../confusion.json", - "field": "actual", - }, + "rev": "HEAD", + "filename": "../confusion.json", + "field": "actual", }, ) assert html_result["../confusion.json"]["data"]["values"] == _update_datapoints( confusion_v2, { REVISION_FIELD: "workspace", + "filename": "../confusion.json", + "field": "actual", }, ) + _update_datapoints( confusion_v1, { REVISION_FIELD: "HEAD", + "filename": "../confusion.json", + "field": "actual", }, ) @@ -434,7 +434,7 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): repo_state = repo_with_config_plots() plots = next(repo_state) - html_path, _, __ = call(capsys) + html_path, json_result, __ = call(capsys) assert os.path.exists(html_path) html_result = extract_vega_specs( @@ -444,15 +444,20 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): "confusion_train_vs_test", ], ) + ble = _update_datapoints( plots["data"]["linear_train.json"], { - REVISION_FIELD: "linear_train.json", + "rev": "workspace", + "filename": "linear_train.json", + "field": "y", }, ) + _update_datapoints( plots["data"]["linear_test.json"], { - REVISION_FIELD: "linear_test.json", + "rev": "workspace", + "filename": "linear_test.json", + "field": "y", }, ) diff --git a/tests/unit/render/test_convert.py b/tests/unit/render/test_convert.py index ea61e857d0..b279f9be24 100644 --- a/tests/unit/render/test_convert.py +++ b/tests/unit/render/test_convert.py @@ -1,4 +1,4 @@ -from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY, VERSION_FIELD +from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY from dvc.render.convert import to_json @@ -10,13 +10,13 @@ def test_to_json_vega(mocker): { "x": 1, "y": 2, - VERSION_FIELD: {"revision": "foo"}, + "rev": "foo", "filename": "foo.json", }, { "x": 2, "y": 1, - VERSION_FIELD: {"revision": "bar"}, + "rev": "bar", "filename": "foo.json", }, ] @@ -31,7 +31,7 @@ def test_to_json_vega(mocker): "x": 1, "y": 2, "filename": "foo.json", - VERSION_FIELD: {"revision": "foo"}, + "rev": "foo", }, ], "bar": [ @@ -39,7 +39,7 @@ def test_to_json_vega(mocker): "x": 2, "y": 1, "filename": "foo.json", - VERSION_FIELD: {"revision": "bar"}, + "rev": "bar", }, ], }, @@ -55,13 +55,13 @@ def test_to_json_vega_split(mocker): { "x": 1, "y": 2, - VERSION_FIELD: {"revision": "foo"}, + "rev": "foo", "filename": "foo.json", }, { "x": 2, "y": 1, - VERSION_FIELD: {"revision": "bar"}, + "rev": "bar", "filename": "foo.json", }, ] @@ -76,7 +76,7 @@ def test_to_json_vega_split(mocker): "x": 1, "y": 2, "filename": "foo.json", - VERSION_FIELD: {"revision": "foo"}, + "rev": "foo", } ], "bar": [ @@ -84,7 +84,7 @@ def test_to_json_vega_split(mocker): "x": 2, "y": 1, "filename": "foo.json", - VERSION_FIELD: {"revision": "bar"}, + "rev": "bar", } ], }, diff --git a/tests/unit/render/test_match.py b/tests/unit/render/test_match.py index 58d4b5c5bb..1e6520bfcb 100644 --- a/tests/unit/render/test_match.py +++ b/tests/unit/render/test_match.py @@ -1,7 +1,6 @@ import pytest 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 @@ -158,20 +157,16 @@ def test_match_renderers(M): renderer = renderer_with_errors[0] assert renderer.datapoints == [ { - VERSION_FIELD: { - "revision": "v1", - "filename": "file.json", - "field": "y", - }, + "rev": "v1", + "filename": "file.json", + "field": "y", "x": 1, "y": 1, }, { - VERSION_FIELD: { - "revision": "v1", - "filename": "file.json", - "field": "y", - }, + "rev": "v1", + "filename": "file.json", + "field": "y", "x": 2, "y": 2, }, diff --git a/tests/unit/render/test_vega_converter.py b/tests/unit/render/test_vega_converter.py index 894b123098..2e07b5eb1a 100644 --- a/tests/unit/render/test_vega_converter.py +++ b/tests/unit/render/test_vega_converter.py @@ -3,7 +3,6 @@ import pytest from dvc.exceptions import DvcException -from dvc.render import VERSION_FIELD from dvc.render.converter.vega import FieldNotFoundError, VegaConverter, _lists @@ -35,20 +34,16 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "step": 0, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", }, { "v": 2, "step": 1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", }, ], {"x": "step", "y": "v", "x_label": "step", "y_label": "v"}, @@ -61,20 +56,16 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, { "v": 2, "v2": 0.2, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, ], {"x": "v", "y": "v2", "x_label": "v", "y_label": "v2"}, @@ -99,20 +90,16 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, { "v": 2, "v2": 0.2, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, ], {"x": "v", "y": "v2", "x_label": "x", "y_label": "y"}, @@ -123,44 +110,36 @@ def test_finding_lists(dictionary, expected_result): {"y": {"f": ["v", "v2"]}}, [ { - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", "dvc_inferred_y_value": 1, "v": 1, "v2": 0.1, "step": 0, }, { - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", "dvc_inferred_y_value": 2, "v": 2, "v2": 0.2, "step": 1, }, { - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", "dvc_inferred_y_value": 0.1, "v2": 0.1, "v": 1, "step": 0, }, { - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", "v": 2, "v2": 0.2, "dvc_inferred_y_value": 0.2, @@ -189,44 +168,36 @@ def test_finding_lists(dictionary, expected_result): "z": 3, "v": 1, "step": 0, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", }, { "dvc_inferred_y_value": 2, "z": 4, "step": 1, "v": 2, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", }, { "dvc_inferred_y_value": 3, "v": 1, "z": 3, "step": 0, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "z", - }, + "rev": "r", + "filename": "f", + "field": "z", }, { "dvc_inferred_y_value": 4, "v": 2, "z": 4, "step": 1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "z", - }, + "rev": "r", + "filename": "f", + "field": "z", }, ], { @@ -247,29 +218,23 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, { "v": 2, "v2": 0.2, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, { "v": 3, "v2": 0.3, - VERSION_FIELD: { - "revision": "r", - "filename": "f2", - "field": "v2", - }, + "rev": "r", + "filename": "f2", + "field": "v2", }, ], {"x": "v", "y": "v2", "x_label": "v", "y_label": "v2"}, @@ -284,44 +249,36 @@ def test_finding_lists(dictionary, expected_result): "v": 1, "v2": 0.1, "step": 0, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", }, { "dvc_inferred_y_value": 2, "v": 2, "v2": 0.2, "step": 1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, + "rev": "r", + "filename": "f", + "field": "v", }, { "dvc_inferred_y_value": 0.1, "v": 1, "v2": 0.1, "step": 0, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, { "dvc_inferred_y_value": 0.2, "v": 2, "v2": 0.2, "step": 1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, ], { @@ -344,32 +301,26 @@ def test_finding_lists(dictionary, expected_result): "v": 1, "v2": 0.1, "v3": 0.01, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, { "dvc_inferred_y_value": 0.01, "v": 1, "v2": 0.1, "v3": 0.01, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v3", - }, + "rev": "r", + "filename": "f", + "field": "v3", }, { "dvc_inferred_y_value": 0.1, "v": 1, "v2": 0.1, - VERSION_FIELD: { - "revision": "r", - "filename": "f2", - "field": "v2", - }, + "rev": "r", + "filename": "f2", + "field": "v2", }, ], { @@ -390,20 +341,16 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, + "rev": "r", + "filename": "f", + "field": "v2", }, { "v": 1, "v2": 0.1, - VERSION_FIELD: { - "revision": "r", - "filename": "f2", - "field": "v2", - }, + "rev": "r", + "filename": "f2", + "field": "v2", }, ], { From d5877f0748a7e10316fcd5a0251e8134e8fbe114 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 19 Sep 2023 14:29:53 +1000 Subject: [PATCH 02/14] add data required for new anchors to renderer properties --- dvc/render/__init__.py | 1 + dvc/render/convert.py | 40 ++++++------ dvc/render/converter/vega.py | 17 ++++- dvc/render/match.py | 18 ++++-- tests/integration/plots/test_plots.py | 2 +- tests/unit/render/test_convert.py | 82 +++++++++++++----------- tests/unit/render/test_match.py | 2 + tests/unit/render/test_vega_converter.py | 56 ++++++++++++++-- 8 files changed, 146 insertions(+), 72 deletions(-) diff --git a/dvc/render/__init__.py b/dvc/render/__init__.py index b5c6c747a8..3937c4a412 100644 --- a/dvc/render/__init__.py +++ b/dvc/render/__init__.py @@ -3,5 +3,6 @@ FILENAME_FIELD = "filename" FIELD_FIELD = "field" REVISIONS_KEY = "revisions" +ANCHORS_Y_DEFN = "anchors_y_defn" TYPE_KEY = "type" SRC_FIELD = "src" diff --git a/dvc/render/convert.py b/dvc/render/convert.py index a61de29162..7f71776645 100644 --- a/dvc/render/convert.py +++ b/dvc/render/convert.py @@ -1,7 +1,12 @@ -from collections import defaultdict from typing import Dict, List, Union -from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY +from dvc.render import ( + ANCHORS_Y_DEFN, + REVISION_FIELD, + REVISIONS_KEY, + SRC_FIELD, + TYPE_KEY, +) from dvc.render.converter.image import ImageConverter from dvc.render.converter.vega import VegaConverter @@ -19,33 +24,26 @@ def _get_converter( raise ValueError(f"Invalid renderer class {renderer_class}") -def _group_by_rev(datapoints): - grouped = defaultdict(list) - for datapoint in datapoints: - rev = datapoint.get(REVISION_FIELD) - grouped[rev].append(datapoint) - return dict(grouped) - - def to_json(renderer, split: bool = False) -> List[Dict]: if renderer.TYPE == "vega": - grouped = _group_by_rev(renderer.datapoints) + if not renderer.datapoints: + return [] if split: content = renderer.get_filled_template( skip_anchors=["data"], as_string=False ) else: content = renderer.get_filled_template(as_string=False) - if grouped: - return [ - { - TYPE_KEY: renderer.TYPE, - REVISIONS_KEY: sorted(grouped.keys()), - "content": content, - "datapoints": grouped, - } - ] - return [] + + return [ + { + ANCHORS_Y_DEFN: renderer.properties.get("anchors_y_defn", {}), + TYPE_KEY: renderer.TYPE, + REVISIONS_KEY: renderer.properties.get("anchor_revs", []), + "content": content, + "datapoints": renderer.datapoints, + } + ] if renderer.TYPE == "image": return [ { diff --git a/dvc/render/converter/vega.py b/dvc/render/converter/vega.py index 4ede2d3d1b..e4e3e01a4f 100644 --- a/dvc/render/converter/vega.py +++ b/dvc/render/converter/vega.py @@ -192,7 +192,7 @@ def infer_x_label(properties): def flat_datapoints(self, revision): # noqa: C901, PLR0912 file2datapoints, properties = self.convert() - props_update = {} + props_update: Dict[str, Union[str, List[Dict[str, str]]]] = {} xs = list(_get_xs(properties, file2datapoints)) @@ -237,6 +237,14 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 else: common_prefix_len = 0 + props_update["anchors_y_defn"] = [ + { + FILENAME_FIELD: _get_short_y_file(y_file, common_prefix_len), + FIELD_FIELD: y_field, + } + for y_file, y_field in ys + ] + for i, (y_file, y_field) in enumerate(ys): if num_xs > 1: x_file, x_field = xs[i] @@ -266,12 +274,11 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 "They have to have same length." ) - y_file_short = y_file[common_prefix_len:].strip("/\\") _update_all( datapoints, update_dict={ REVISION_FIELD: revision, - FILENAME_FIELD: y_file_short, + FILENAME_FIELD: _get_short_y_file(y_file, common_prefix_len), FIELD_FIELD: y_field, }, ) @@ -304,6 +311,10 @@ def convert( return datapoints, properties +def _get_short_y_file(y_file, common_prefix_len): + return y_file[common_prefix_len:].strip("/\\") + + def _update_from_field( target_datapoints: List[Dict], field: str, diff --git a/dvc/render/match.py b/dvc/render/match.py index f7fb6e7a03..0985ced4e6 100644 --- a/dvc/render/match.py +++ b/dvc/render/match.py @@ -80,7 +80,7 @@ def match_defs_renderers( # noqa: C901, PLR0912 for plot_id, group in plots_data.group_definitions().items(): plot_datapoints: List[Dict] = [] props = _squash_plots_properties(group) - final_props: Dict = {} + first_props: Dict = {} def_errors: Dict[str, Exception] = {} src_errors: DefaultDict[str, Dict[str, Exception]] = defaultdict(dict) @@ -90,6 +90,7 @@ def match_defs_renderers( # noqa: C901, PLR0912 if templates_dir is not None: props["template_dir"] = templates_dir + revs = [] for rev, inner_id, plot_definition in group: plot_sources = infer_data_sources(inner_id, plot_definition) definitions_data = plots_data.get_definition_data(plot_sources, rev) @@ -100,6 +101,8 @@ def match_defs_renderers( # noqa: C901, PLR0912 else: renderer_cls = VegaRenderer renderer_id = plot_id + if rev not in revs: + revs.append(rev) converter = _get_converter(renderer_cls, inner_id, props, definitions_data) @@ -114,14 +117,17 @@ def match_defs_renderers( # noqa: C901, PLR0912 def_errors[rev] = e continue - if not final_props and rev_props: - final_props = rev_props + if not first_props and rev_props: + first_props = rev_props plot_datapoints.extend(dps) - if "title" not in final_props: - final_props["title"] = renderer_id + if "title" not in first_props: + first_props["title"] = renderer_id + + if revs: + first_props["anchor_revs"] = revs if renderer_cls is not None: - renderer = renderer_cls(plot_datapoints, renderer_id, **final_props) + renderer = renderer_cls(plot_datapoints, renderer_id, **first_props) renderers.append(RendererWithErrors(renderer, dict(src_errors), def_errors)) return renderers diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 6a46534047..c3afdc5017 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -112,7 +112,7 @@ def verify_vega( assert set(j[0]["revisions"]) == set(versions) assert json_result[0]["datapoints"] == split_json_result[0]["datapoints"] - assert set(versions) == set(json_result[0]["datapoints"].keys()) + assert set(versions) == set(json_result[0]["revisions"]) assert json_result[0]["content"]["data"]["values"] assert html_result["data"]["values"] diff --git a/tests/unit/render/test_convert.py b/tests/unit/render/test_convert.py index b279f9be24..26bb146a10 100644 --- a/tests/unit/render/test_convert.py +++ b/tests/unit/render/test_convert.py @@ -1,10 +1,20 @@ -from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY +from dvc.render import ( + ANCHORS_Y_DEFN, + REVISION_FIELD, + REVISIONS_KEY, + SRC_FIELD, + TYPE_KEY, +) from dvc.render.convert import to_json def test_to_json_vega(mocker): vega_renderer = mocker.MagicMock() vega_renderer.TYPE = "vega" + vega_renderer.properties = { + ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], + "anchor_revs": ["bar", "foo"], + } vega_renderer.get_filled_template.return_value = {"this": "is vega"} vega_renderer.datapoints = [ { @@ -22,27 +32,24 @@ def test_to_json_vega(mocker): ] result = to_json(vega_renderer) assert result[0] == { + ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], TYPE_KEY: vega_renderer.TYPE, REVISIONS_KEY: ["bar", "foo"], "content": {"this": "is vega"}, - "datapoints": { - "foo": [ - { - "x": 1, - "y": 2, - "filename": "foo.json", - "rev": "foo", - }, - ], - "bar": [ - { - "x": 2, - "y": 1, - "filename": "foo.json", - "rev": "bar", - }, - ], - }, + "datapoints": [ + { + "x": 1, + "y": 2, + "filename": "foo.json", + "rev": "foo", + }, + { + "x": 2, + "y": 1, + "filename": "foo.json", + "rev": "bar", + }, + ], } vega_renderer.get_filled_template.assert_called() @@ -51,6 +58,10 @@ def test_to_json_vega_split(mocker): vega_renderer = mocker.MagicMock() vega_renderer.TYPE = "vega" vega_renderer.get_filled_template.return_value = {"this": "is split vega"} + vega_renderer.properties = { + ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], + "anchor_revs": ["bar", "foo"], + } vega_renderer.datapoints = [ { "x": 1, @@ -67,27 +78,24 @@ def test_to_json_vega_split(mocker): ] result = to_json(vega_renderer, split=True) assert result[0] == { + ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], TYPE_KEY: vega_renderer.TYPE, REVISIONS_KEY: ["bar", "foo"], "content": {"this": "is split vega"}, - "datapoints": { - "foo": [ - { - "x": 1, - "y": 2, - "filename": "foo.json", - "rev": "foo", - } - ], - "bar": [ - { - "x": 2, - "y": 1, - "filename": "foo.json", - "rev": "bar", - } - ], - }, + "datapoints": [ + { + "x": 1, + "y": 2, + "filename": "foo.json", + "rev": "foo", + }, + { + "x": 2, + "y": 1, + "filename": "foo.json", + "rev": "bar", + }, + ], } vega_renderer.get_filled_template.assert_called_with( as_string=False, skip_anchors=["data"] diff --git a/tests/unit/render/test_match.py b/tests/unit/render/test_match.py index 1e6520bfcb..a69e35b6ec 100644 --- a/tests/unit/render/test_match.py +++ b/tests/unit/render/test_match.py @@ -172,6 +172,8 @@ def test_match_renderers(M): }, ] assert renderer.properties == { + "anchors_y_defn": [{"filename": "file.json", "field": "y"}], + "anchor_revs": ["v1", "revision_with_no_data"], "title": "plot_id_1", "x": "x", "y": "y", diff --git a/tests/unit/render/test_vega_converter.py b/tests/unit/render/test_vega_converter.py index 2e07b5eb1a..4eb80a09fa 100644 --- a/tests/unit/render/test_vega_converter.py +++ b/tests/unit/render/test_vega_converter.py @@ -46,7 +46,13 @@ def test_finding_lists(dictionary, expected_result): "field": "v", }, ], - {"x": "step", "y": "v", "x_label": "step", "y_label": "v"}, + { + "anchors_y_defn": [{"filename": "f", "field": "v"}], + "x": "step", + "y": "v", + "x_label": "step", + "y_label": "v", + }, id="default_x_y", ), pytest.param( @@ -68,7 +74,13 @@ def test_finding_lists(dictionary, expected_result): "field": "v2", }, ], - {"x": "v", "y": "v2", "x_label": "v", "y_label": "v2"}, + { + "anchors_y_defn": [{"filename": "f", "field": "v2"}], + "x": "v", + "y": "v2", + "x_label": "v", + "y_label": "v2", + }, id="choose_x_y", ), pytest.param( @@ -102,7 +114,13 @@ def test_finding_lists(dictionary, expected_result): "field": "v2", }, ], - {"x": "v", "y": "v2", "x_label": "x", "y_label": "y"}, + { + "anchors_y_defn": [{"filename": "f", "field": "v2"}], + "x": "v", + "y": "v2", + "x_label": "x", + "y_label": "y", + }, id="find_in_nested_structure", ), pytest.param( @@ -147,6 +165,10 @@ def test_finding_lists(dictionary, expected_result): }, ], { + "anchors_y_defn": [ + {"filename": "f", "field": "v"}, + {"filename": "f", "field": "v2"}, + ], "x": "step", "y": "dvc_inferred_y_value", "y_label": "y", @@ -201,6 +223,10 @@ def test_finding_lists(dictionary, expected_result): }, ], { + "anchors_y_defn": [ + {"filename": "f", "field": "v"}, + {"filename": "f", "field": "z"}, + ], "x": "step", "y": "dvc_inferred_y_value", "y_label": "y", @@ -237,7 +263,16 @@ def test_finding_lists(dictionary, expected_result): "field": "v2", }, ], - {"x": "v", "y": "v2", "x_label": "v", "y_label": "v2"}, + { + "anchors_y_defn": [ + {"filename": "f", "field": "v2"}, + {"filename": "f2", "field": "v2"}, + ], + "x": "v", + "y": "v2", + "x_label": "v", + "y_label": "v2", + }, id="multi_file_json", ), pytest.param( @@ -282,6 +317,10 @@ def test_finding_lists(dictionary, expected_result): }, ], { + "anchors_y_defn": [ + {"filename": "f", "field": "v"}, + {"filename": "f", "field": "v2"}, + ], "x": "step", "y": "dvc_inferred_y_value", "x_label": "step", @@ -324,6 +363,11 @@ def test_finding_lists(dictionary, expected_result): }, ], { + "anchors_y_defn": [ + {"filename": "f", "field": "v2"}, + {"filename": "f", "field": "v3"}, + {"filename": "f2", "field": "v2"}, + ], "x": "v", "y": "dvc_inferred_y_value", "x_label": "v", @@ -354,6 +398,10 @@ def test_finding_lists(dictionary, expected_result): }, ], { + "anchors_y_defn": [ + {"filename": "f", "field": "v2"}, + {"filename": "f2", "field": "v2"}, + ], "x": "v", "y": "v2", "x_label": "v", From 3c44e59f1b529fd1fc532310617034557847b7d3 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 22 Sep 2023 11:50:55 +1000 Subject: [PATCH 03/14] feed dvc-render updates into API output --- dvc/render/__init__.py | 2 +- dvc/render/convert.py | 19 ++--- pyproject.toml | 3 +- tests/integration/plots/test_plots.py | 43 +++++------ tests/unit/render/test_convert.py | 104 ++++++++++---------------- 5 files changed, 68 insertions(+), 103 deletions(-) diff --git a/dvc/render/__init__.py b/dvc/render/__init__.py index 3937c4a412..9c732531d4 100644 --- a/dvc/render/__init__.py +++ b/dvc/render/__init__.py @@ -3,6 +3,6 @@ FILENAME_FIELD = "filename" FIELD_FIELD = "field" REVISIONS_KEY = "revisions" -ANCHORS_Y_DEFN = "anchors_y_defn" +ANCHOR_DEFINITIONS = "anchor_definitions" TYPE_KEY = "type" SRC_FIELD = "src" diff --git a/dvc/render/convert.py b/dvc/render/convert.py index 7f71776645..182320b6e1 100644 --- a/dvc/render/convert.py +++ b/dvc/render/convert.py @@ -1,12 +1,6 @@ from typing import Dict, List, Union -from dvc.render import ( - ANCHORS_Y_DEFN, - REVISION_FIELD, - REVISIONS_KEY, - SRC_FIELD, - TYPE_KEY, -) +from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY from dvc.render.converter.image import ImageConverter from dvc.render.converter.vega import VegaConverter @@ -28,20 +22,19 @@ def to_json(renderer, split: bool = False) -> List[Dict]: if renderer.TYPE == "vega": if not renderer.datapoints: return [] + revs = renderer.get_revs() if split: - content = renderer.get_filled_template( - skip_anchors=["data"], as_string=False - ) + content, split_content = renderer.get_partial_filled_template() else: content = renderer.get_filled_template(as_string=False) + split_content = {} return [ { - ANCHORS_Y_DEFN: renderer.properties.get("anchors_y_defn", {}), TYPE_KEY: renderer.TYPE, - REVISIONS_KEY: renderer.properties.get("anchor_revs", []), + REVISIONS_KEY: revs, "content": content, - "datapoints": renderer.datapoints, + **split_content, } ] if renderer.TYPE == "image": diff --git a/pyproject.toml b/pyproject.toml index 6b397db22f..c5402434c8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ classifiers = [ "Programming Language :: Python :: 3.11", ] dynamic = ["version"] + dependencies = [ "colorama>=0.3.9", "configobj>=5.0.6", @@ -36,7 +37,7 @@ dependencies = [ "dpath<3,>=2.1.0", "dvc-data>=2.22.1,<2.23.0", "dvc-http>=2.29.0", - "dvc-render>=0.3.1,<1", + "dvc-render>=1.0.0,<2", "dvc-studio-client>=0.17.1,<1", "dvc-task>=0.3.0,<1", "flatten_dict<1,>=0.4.1", diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index c3afdc5017..1d841cfa6e 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -111,22 +111,26 @@ def verify_vega( assert j[0]["type"] == "vega" assert set(j[0]["revisions"]) == set(versions) - assert json_result[0]["datapoints"] == split_json_result[0]["datapoints"] + assert ( + json_result[0]["content"]["data"]["values"] + == split_json_result[0]["anchor_definitions"][""] + ) assert set(versions) == set(json_result[0]["revisions"]) assert json_result[0]["content"]["data"]["values"] assert html_result["data"]["values"] - assert split_json_result[0]["content"]["data"]["values"] == "" + assert "" in split_json_result[0]["content"] def _assert_templates_equal(html_template, filled_template, split_template): - # besides data, json and split json should be equal - path = ["data", "values"] + # besides split anchors, json and split json should be equal + paths = [["data", "values"], ["encoding", "color"]] tmp1 = deepcopy(html_template) tmp2 = deepcopy(filled_template) - tmp3 = deepcopy(split_template) - dpath.set(tmp1, path, {}) - dpath.set(tmp2, path, {}) - dpath.set(tmp3, path, {}) + tmp3 = deepcopy(json.loads(split_template)) + for path in paths: + dpath.set(tmp1, path, {}) + dpath.set(tmp2, path, {}) + dpath.set(tmp3, path, {}) assert tmp1 == tmp2 == tmp3 @@ -194,16 +198,12 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ linear_v1, { "rev": "workspace", - "filename": "linear.json", - "field": "y", }, ) assert html_result["linear.json"]["data"]["values"] == _update_datapoints( linear_v1, { REVISION_FIELD: "workspace", - "filename": "linear.json", - "field": "y", }, ) assert json_data["confusion.json"][0]["content"]["data"][ @@ -278,30 +278,22 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ linear_v2, { "rev": "workspace", - "filename": "../linear.json", - "field": "y", }, ) + _update_datapoints( linear_v1, { "rev": "HEAD", - "filename": "../linear.json", - "field": "y", }, ) assert html_result["../linear.json"]["data"]["values"] == _update_datapoints( linear_v2, { REVISION_FIELD: "workspace", - "filename": "../linear.json", - "field": "y", }, ) + _update_datapoints( linear_v1, { REVISION_FIELD: "HEAD", - "filename": "../linear.json", - "field": "y", }, ) assert json_data["../confusion.json"][0]["content"]["data"][ @@ -434,7 +426,7 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): repo_state = repo_with_config_plots() plots = next(repo_state) - html_path, json_result, __ = call(capsys) + html_path, _, split_json_result = call(capsys) assert os.path.exists(html_path) html_result = extract_vega_specs( @@ -450,19 +442,22 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): { "rev": "workspace", "filename": "linear_train.json", - "field": "y", }, ) + _update_datapoints( plots["data"]["linear_test.json"], { "rev": "workspace", "filename": "linear_test.json", - "field": "y", }, ) assert html_result["linear_train_vs_test"]["data"]["values"] == ble - # TODO check json results once vscode is able to handle flexible plots + assert ( + split_json_result["data"]["linear_train_vs_test"][0]["anchor_definitions"][ + "" + ] + == ble + ) @pytest.mark.vscode diff --git a/tests/unit/render/test_convert.py b/tests/unit/render/test_convert.py index 26bb146a10..8fc56b0575 100644 --- a/tests/unit/render/test_convert.py +++ b/tests/unit/render/test_convert.py @@ -1,5 +1,9 @@ +import json + +import pytest + from dvc.render import ( - ANCHORS_Y_DEFN, + ANCHOR_DEFINITIONS, REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, @@ -11,95 +15,67 @@ def test_to_json_vega(mocker): vega_renderer = mocker.MagicMock() vega_renderer.TYPE = "vega" - vega_renderer.properties = { - ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], - "anchor_revs": ["bar", "foo"], - } + vega_renderer.get_revs.return_value = ["bar", "foo"] vega_renderer.get_filled_template.return_value = {"this": "is vega"} - vega_renderer.datapoints = [ - { - "x": 1, - "y": 2, - "rev": "foo", - "filename": "foo.json", - }, - { - "x": 2, - "y": 1, - "rev": "bar", - "filename": "foo.json", - }, - ] result = to_json(vega_renderer) assert result[0] == { - ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], TYPE_KEY: vega_renderer.TYPE, REVISIONS_KEY: ["bar", "foo"], "content": {"this": "is vega"}, - "datapoints": [ - { - "x": 1, - "y": 2, - "filename": "foo.json", - "rev": "foo", - }, - { - "x": 2, - "y": 1, - "filename": "foo.json", - "rev": "bar", - }, - ], } vega_renderer.get_filled_template.assert_called() +@pytest.mark.vscode def test_to_json_vega_split(mocker): - vega_renderer = mocker.MagicMock() - vega_renderer.TYPE = "vega" - vega_renderer.get_filled_template.return_value = {"this": "is split vega"} - vega_renderer.properties = { - ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], - "anchor_revs": ["bar", "foo"], - } - vega_renderer.datapoints = [ - { - "x": 1, - "y": 2, - "rev": "foo", - "filename": "foo.json", - }, + revs = ["bar", "foo"] + content = json.dumps( { - "x": 2, - "y": 1, - "rev": "bar", - "filename": "foo.json", + "this": "is split vega", + "encoding": {"color": ""}, + "data": {"values": ""}, + } + ) + anchor_definitions = { + "": { + "field": "rev", + "scale": { + "domain": revs, + "range": ["#ff0000", "#00ff00"], + }, }, - ] - result = to_json(vega_renderer, split=True) - assert result[0] == { - ANCHORS_Y_DEFN: [{"filename": "foo.json", "field": "y"}], - TYPE_KEY: vega_renderer.TYPE, - REVISIONS_KEY: ["bar", "foo"], - "content": {"this": "is split vega"}, - "datapoints": [ + "": [ { "x": 1, "y": 2, - "filename": "foo.json", "rev": "foo", + "filename": "foo.json", }, { "x": 2, "y": 1, - "filename": "foo.json", "rev": "bar", + "filename": "foo.json", }, ], } - vega_renderer.get_filled_template.assert_called_with( - as_string=False, skip_anchors=["data"] + + vega_renderer = mocker.MagicMock() + vega_renderer.TYPE = "vega" + vega_renderer.get_partial_filled_template.return_value = ( + content, + {ANCHOR_DEFINITIONS: anchor_definitions}, ) + vega_renderer.get_revs.return_value = ["bar", "foo"] + + result = to_json(vega_renderer, split=True) + assert result[0] == { + ANCHOR_DEFINITIONS: anchor_definitions, + TYPE_KEY: vega_renderer.TYPE, + REVISIONS_KEY: revs, + "content": content, + } + vega_renderer.get_partial_filled_template.assert_called_once() def test_to_json_image(mocker): From d52d7d87eedd0bea825cb69411916bf66d9c5448 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 22 Sep 2023 12:22:48 +1000 Subject: [PATCH 04/14] standardise names --- dvc/render/__init__.py | 12 ++++++------ dvc/render/convert.py | 8 ++++---- dvc/render/converter/image.py | 8 ++++---- dvc/render/converter/vega.py | 22 +++++++++++----------- tests/integration/plots/test_plots.py | 12 ++++++------ tests/unit/render/test_convert.py | 20 +++++++------------- tests/unit/render/test_image_converter.py | 14 +++++++------- tests/unit/render/test_match.py | 2 +- tests/unit/render/test_vega_converter.py | 18 +++++++++--------- 9 files changed, 55 insertions(+), 61 deletions(-) diff --git a/dvc/render/__init__.py b/dvc/render/__init__.py index 9c732531d4..a3ee972fb9 100644 --- a/dvc/render/__init__.py +++ b/dvc/render/__init__.py @@ -1,8 +1,8 @@ -INDEX_FIELD = "step" -REVISION_FIELD = "rev" -FILENAME_FIELD = "filename" -FIELD_FIELD = "field" -REVISIONS_KEY = "revisions" +INDEX = "step" +REVISION = "rev" +FILENAME = "filename" +FIELD = "field" +REVISIONS = "revisions" ANCHOR_DEFINITIONS = "anchor_definitions" TYPE_KEY = "type" -SRC_FIELD = "src" +SRC = "src" diff --git a/dvc/render/convert.py b/dvc/render/convert.py index 182320b6e1..b5466839b3 100644 --- a/dvc/render/convert.py +++ b/dvc/render/convert.py @@ -1,6 +1,6 @@ from typing import Dict, List, Union -from dvc.render import REVISION_FIELD, REVISIONS_KEY, SRC_FIELD, TYPE_KEY +from dvc.render import REVISION, REVISIONS, SRC, TYPE_KEY from dvc.render.converter.image import ImageConverter from dvc.render.converter.vega import VegaConverter @@ -32,7 +32,7 @@ def to_json(renderer, split: bool = False) -> List[Dict]: return [ { TYPE_KEY: renderer.TYPE, - REVISIONS_KEY: revs, + REVISIONS: revs, "content": content, **split_content, } @@ -41,8 +41,8 @@ def to_json(renderer, split: bool = False) -> List[Dict]: return [ { TYPE_KEY: renderer.TYPE, - REVISIONS_KEY: [datapoint.get(REVISION_FIELD)], - "url": datapoint.get(SRC_FIELD), + REVISIONS: [datapoint.get(REVISION)], + "url": datapoint.get(SRC), } for datapoint in renderer.datapoints ] diff --git a/dvc/render/converter/image.py b/dvc/render/converter/image.py index fba390b306..fed3a02f4c 100644 --- a/dvc/render/converter/image.py +++ b/dvc/render/converter/image.py @@ -2,7 +2,7 @@ import os from typing import TYPE_CHECKING, Any, Dict, List, Tuple -from dvc.render import FILENAME_FIELD, REVISION_FIELD, SRC_FIELD +from dvc.render import FILENAME, REVISION, SRC from . import Converter @@ -58,9 +58,9 @@ def flat_datapoints(self, revision: str) -> Tuple[List[Dict], Dict]: else: src = self._encode_image(image_data) datapoint = { - REVISION_FIELD: revision, - FILENAME_FIELD: filename, - SRC_FIELD: src, + REVISION: revision, + FILENAME: filename, + SRC: src, } datapoints.append(datapoint) return datapoints, properties diff --git a/dvc/render/converter/vega.py b/dvc/render/converter/vega.py index e4e3e01a4f..6838dcdb49 100644 --- a/dvc/render/converter/vega.py +++ b/dvc/render/converter/vega.py @@ -4,7 +4,7 @@ from funcy import first, last from dvc.exceptions import DvcException -from dvc.render import FIELD_FIELD, FILENAME_FIELD, INDEX_FIELD, REVISION_FIELD +from dvc.render import FIELD, FILENAME, INDEX, REVISION from . import Converter @@ -182,7 +182,7 @@ def infer_x_label(properties): x = properties.get("x", None) if not isinstance(x, dict): - return INDEX_FIELD + return INDEX fields = {field for _, field in _file_field(x)} if len(fields) == 1: @@ -200,7 +200,7 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 if not xs: x_file, x_field = ( None, - INDEX_FIELD, + INDEX, ) else: x_file, x_field = xs[0] @@ -237,10 +237,10 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 else: common_prefix_len = 0 - props_update["anchors_y_defn"] = [ + props_update["anchors_y_definitions"] = [ { - FILENAME_FIELD: _get_short_y_file(y_file, common_prefix_len), - FIELD_FIELD: y_field, + FILENAME: _get_short_y_file(y_file, common_prefix_len), + FIELD: y_field, } for y_file, y_field in ys ] @@ -257,8 +257,8 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 source_field=y_field, ) - if x_field == INDEX_FIELD and x_file is None: - _update_from_index(datapoints, INDEX_FIELD) + if x_field == INDEX and x_file is None: + _update_from_index(datapoints, INDEX) else: x_datapoints = file2datapoints.get(x_file, []) try: @@ -277,9 +277,9 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 _update_all( datapoints, update_dict={ - REVISION_FIELD: revision, - FILENAME_FIELD: _get_short_y_file(y_file, common_prefix_len), - FIELD_FIELD: y_field, + REVISION: revision, + FILENAME: _get_short_y_file(y_file, common_prefix_len), + FIELD: y_field, }, ) diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 1d841cfa6e..883bf9a4a5 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -11,7 +11,7 @@ from funcy import first from dvc.cli import main -from dvc.render import REVISION_FIELD +from dvc.render import REVISION JSON_OUT = "vis_data" @@ -203,7 +203,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ assert html_result["linear.json"]["data"]["values"] == _update_datapoints( linear_v1, { - REVISION_FIELD: "workspace", + REVISION: "workspace", }, ) assert json_data["confusion.json"][0]["content"]["data"][ @@ -288,12 +288,12 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ assert html_result["../linear.json"]["data"]["values"] == _update_datapoints( linear_v2, { - REVISION_FIELD: "workspace", + REVISION: "workspace", }, ) + _update_datapoints( linear_v1, { - REVISION_FIELD: "HEAD", + REVISION: "HEAD", }, ) assert json_data["../confusion.json"][0]["content"]["data"][ @@ -316,14 +316,14 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ assert html_result["../confusion.json"]["data"]["values"] == _update_datapoints( confusion_v2, { - REVISION_FIELD: "workspace", + REVISION: "workspace", "filename": "../confusion.json", "field": "actual", }, ) + _update_datapoints( confusion_v1, { - REVISION_FIELD: "HEAD", + REVISION: "HEAD", "filename": "../confusion.json", "field": "actual", }, diff --git a/tests/unit/render/test_convert.py b/tests/unit/render/test_convert.py index 8fc56b0575..204314e95d 100644 --- a/tests/unit/render/test_convert.py +++ b/tests/unit/render/test_convert.py @@ -2,13 +2,7 @@ import pytest -from dvc.render import ( - ANCHOR_DEFINITIONS, - REVISION_FIELD, - REVISIONS_KEY, - SRC_FIELD, - TYPE_KEY, -) +from dvc.render import ANCHOR_DEFINITIONS, REVISION, REVISIONS, SRC, TYPE_KEY from dvc.render.convert import to_json @@ -20,7 +14,7 @@ def test_to_json_vega(mocker): result = to_json(vega_renderer) assert result[0] == { TYPE_KEY: vega_renderer.TYPE, - REVISIONS_KEY: ["bar", "foo"], + REVISIONS: ["bar", "foo"], "content": {"this": "is vega"}, } vega_renderer.get_filled_template.assert_called() @@ -72,7 +66,7 @@ def test_to_json_vega_split(mocker): assert result[0] == { ANCHOR_DEFINITIONS: anchor_definitions, TYPE_KEY: vega_renderer.TYPE, - REVISIONS_KEY: revs, + REVISIONS: revs, "content": content, } vega_renderer.get_partial_filled_template.assert_called_once() @@ -82,12 +76,12 @@ def test_to_json_image(mocker): image_renderer = mocker.MagicMock() image_renderer.TYPE = "image" image_renderer.datapoints = [ - {SRC_FIELD: "contentfoo", REVISION_FIELD: "foo"}, - {SRC_FIELD: "contentbar", REVISION_FIELD: "bar"}, + {SRC: "contentfoo", REVISION: "foo"}, + {SRC: "contentbar", REVISION: "bar"}, ] result = to_json(image_renderer) assert result[0] == { - "url": image_renderer.datapoints[0].get(SRC_FIELD), - REVISIONS_KEY: [image_renderer.datapoints[0].get(REVISION_FIELD)], + "url": image_renderer.datapoints[0].get(SRC), + REVISIONS: [image_renderer.datapoints[0].get(REVISION)], TYPE_KEY: image_renderer.TYPE, } diff --git a/tests/unit/render/test_image_converter.py b/tests/unit/render/test_image_converter.py index 97497d8c84..3081203078 100644 --- a/tests/unit/render/test_image_converter.py +++ b/tests/unit/render/test_image_converter.py @@ -1,4 +1,4 @@ -from dvc.render import REVISION_FIELD, SRC_FIELD +from dvc.render import REVISION, SRC from dvc.render.converter.image import ImageConverter @@ -8,9 +8,9 @@ def test_image_converter_no_out(): datapoints, _ = converter.flat_datapoints("r") assert datapoints[0] == { - REVISION_FIELD: "r", + REVISION: "r", "filename": "image.png", - SRC_FIELD: converter._encode_image(b"content"), + SRC: converter._encode_image(b"content"), } @@ -21,9 +21,9 @@ def test_image_converter_with_out(tmp_dir): datapoints, _ = converter.flat_datapoints("r") assert datapoints[0] == { - REVISION_FIELD: "r", + REVISION: "r", "filename": "image.png", - SRC_FIELD: str(tmp_dir / "foo" / "r_image.png"), + SRC: str(tmp_dir / "foo" / "r_image.png"), } assert (tmp_dir / "foo" / "r_image.png").read_bytes() == b"content" @@ -37,9 +37,9 @@ def test_image_converter_with_slash_in_revision(tmp_dir): datapoints, _ = converter.flat_datapoints("feature/r") assert datapoints[0] == { - REVISION_FIELD: "feature/r", + REVISION: "feature/r", "filename": "image.png", - SRC_FIELD: str(tmp_dir / "foo" / "feature_r_image.png"), + SRC: str(tmp_dir / "foo" / "feature_r_image.png"), } assert (tmp_dir / "foo" / "feature_r_image.png").read_bytes() == b"content" diff --git a/tests/unit/render/test_match.py b/tests/unit/render/test_match.py index a69e35b6ec..c590f6c836 100644 --- a/tests/unit/render/test_match.py +++ b/tests/unit/render/test_match.py @@ -172,7 +172,7 @@ def test_match_renderers(M): }, ] assert renderer.properties == { - "anchors_y_defn": [{"filename": "file.json", "field": "y"}], + "anchors_y_definitions": [{"filename": "file.json", "field": "y"}], "anchor_revs": ["v1", "revision_with_no_data"], "title": "plot_id_1", "x": "x", diff --git a/tests/unit/render/test_vega_converter.py b/tests/unit/render/test_vega_converter.py index 4eb80a09fa..4710dbc140 100644 --- a/tests/unit/render/test_vega_converter.py +++ b/tests/unit/render/test_vega_converter.py @@ -47,7 +47,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [{"filename": "f", "field": "v"}], + "anchors_y_definitions": [{"filename": "f", "field": "v"}], "x": "step", "y": "v", "x_label": "step", @@ -75,7 +75,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [{"filename": "f", "field": "v2"}], + "anchors_y_definitions": [{"filename": "f", "field": "v2"}], "x": "v", "y": "v2", "x_label": "v", @@ -115,7 +115,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [{"filename": "f", "field": "v2"}], + "anchors_y_definitions": [{"filename": "f", "field": "v2"}], "x": "v", "y": "v2", "x_label": "x", @@ -165,7 +165,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [ + "anchors_y_definitions": [ {"filename": "f", "field": "v"}, {"filename": "f", "field": "v2"}, ], @@ -223,7 +223,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [ + "anchors_y_definitions": [ {"filename": "f", "field": "v"}, {"filename": "f", "field": "z"}, ], @@ -264,7 +264,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [ + "anchors_y_definitions": [ {"filename": "f", "field": "v2"}, {"filename": "f2", "field": "v2"}, ], @@ -317,7 +317,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [ + "anchors_y_definitions": [ {"filename": "f", "field": "v"}, {"filename": "f", "field": "v2"}, ], @@ -363,7 +363,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [ + "anchors_y_definitions": [ {"filename": "f", "field": "v2"}, {"filename": "f", "field": "v3"}, {"filename": "f2", "field": "v2"}, @@ -398,7 +398,7 @@ def test_finding_lists(dictionary, expected_result): }, ], { - "anchors_y_defn": [ + "anchors_y_definitions": [ {"filename": "f", "field": "v2"}, {"filename": "f2", "field": "v2"}, ], From 00d772029ef12d72746db2de593c43869e0efc09 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Wed, 27 Sep 2023 09:51:39 +1000 Subject: [PATCH 05/14] update --json --split api with new data types (send split anchor definitions as strings) --- tests/integration/plots/test_plots.py | 69 +++++++++++++++++---------- 1 file changed, 43 insertions(+), 26 deletions(-) diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 883bf9a4a5..68840f3690 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -102,6 +102,8 @@ def verify_vega( html_result, json_result, split_json_result, + x_label, + y_label, ): if isinstance(versions, str): versions = [versions] @@ -111,22 +113,31 @@ def verify_vega( assert j[0]["type"] == "vega" assert set(j[0]["revisions"]) == set(versions) - assert ( - json_result[0]["content"]["data"]["values"] - == split_json_result[0]["anchor_definitions"][""] + assert json_result[0]["content"]["data"]["values"] == json.loads( + split_json_result[0]["anchor_definitions"][""] ) assert set(versions) == set(json_result[0]["revisions"]) assert json_result[0]["content"]["data"]["values"] assert html_result["data"]["values"] assert "" in split_json_result[0]["content"] + assert "" in split_json_result[0]["content"] + assert "" in split_json_result[0]["content"] - def _assert_templates_equal(html_template, filled_template, split_template): + def _assert_templates_equal( + html_template, filled_template, split_template, x_label, y_label + ): # besides split anchors, json and split json should be equal paths = [["data", "values"], ["encoding", "color"]] tmp1 = deepcopy(html_template) tmp2 = deepcopy(filled_template) - tmp3 = deepcopy(json.loads(split_template)) + tmp3 = deepcopy(split_template) + tmp3 = json.loads( + tmp3.replace("", x_label).replace( + "", y_label + ) + ) + for path in paths: dpath.set(tmp1, path, {}) dpath.set(tmp2, path, {}) @@ -135,7 +146,11 @@ def _assert_templates_equal(html_template, filled_template, split_template): assert tmp1 == tmp2 == tmp3 _assert_templates_equal( - html_result, json_result[0]["content"], split_json_result[0]["content"] + html_result, + json_result[0]["content"], + split_json_result[0]["content"], + x_label, + y_label, ) @@ -212,26 +227,27 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ confusion_v1, { "rev": "workspace", - "filename": "confusion.json", - "field": "actual", }, ) assert html_result["confusion.json"]["data"]["values"] == _update_datapoints( confusion_v1, { "rev": "workspace", - "filename": "confusion.json", - "field": "actual", }, ) verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_data) - for plot in ["linear.json", "confusion.json"]: + for plot, x_label, y_label in [ + ("linear.json", "x", "y"), + ("confusion.json", "predicted", "actual"), + ]: verify_vega( "workspace", html_result[plot], json_data[plot], split_json_data[plot], + x_label, + y_label, ) verify_vega_props("confusion.json", json_data, **confusion_props) @@ -250,12 +266,17 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ 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"]: + for plot, x_label, y_label in [ + ("linear.json", "x", "y"), + ("confusion.json", "predicted", "actual"), + ]: verify_vega( ["HEAD", "workspace"], html_result[plot], json_data[plot], split_json_data[plot], + x_label, + y_label, ) verify_vega_props("confusion.json", json_data, **confusion_props) path = tmp_dir / "subdir" @@ -302,42 +323,36 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ confusion_v2, { "rev": "workspace", - "filename": "../confusion.json", - "field": "actual", }, ) + _update_datapoints( confusion_v1, { "rev": "HEAD", - "filename": "../confusion.json", - "field": "actual", }, ) assert html_result["../confusion.json"]["data"]["values"] == _update_datapoints( confusion_v2, { REVISION: "workspace", - "filename": "../confusion.json", - "field": "actual", }, ) + _update_datapoints( confusion_v1, { REVISION: "HEAD", - "filename": "../confusion.json", - "field": "actual", }, ) - for plot in [ - "../linear.json", - "../confusion.json", + for plot, x_label, y_label in [ + ("../linear.json", "x", "y"), + ("../confusion.json", "predicted", "actual"), ]: verify_vega( ["HEAD", "workspace"], html_result[plot], json_data[plot], split_json_data[plot], + x_label, + y_label, ) verify_image( path, @@ -453,9 +468,11 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): assert html_result["linear_train_vs_test"]["data"]["values"] == ble assert ( - split_json_result["data"]["linear_train_vs_test"][0]["anchor_definitions"][ - "" - ] + json.loads( + split_json_result["data"]["linear_train_vs_test"][0]["anchor_definitions"][ + "" + ] + ) == ble ) From ebb4cba41d5e404fc41adc5829d26929d3d6dea9 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Wed, 27 Sep 2023 12:35:10 +1000 Subject: [PATCH 06/14] do not use deepcopy for a string --- tests/integration/plots/test_plots.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 68840f3690..cd3d57296a 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -131,13 +131,11 @@ def _assert_templates_equal( paths = [["data", "values"], ["encoding", "color"]] tmp1 = deepcopy(html_template) tmp2 = deepcopy(filled_template) - tmp3 = deepcopy(split_template) tmp3 = json.loads( - tmp3.replace("", x_label).replace( - "", y_label - ) + split_template[:] + .replace("", x_label) + .replace("", y_label) ) - for path in paths: dpath.set(tmp1, path, {}) dpath.set(tmp2, path, {}) From 769f3a5cae3e1bcd1b91e893301928d333a34236 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 3 Oct 2023 13:11:06 +1100 Subject: [PATCH 07/14] accomodate zoom and pan anchor --- tests/integration/plots/test_plots.py | 36 +++++++++++++++++++-------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index cd3d57296a..18d27eafdc 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -102,6 +102,7 @@ def verify_vega( html_result, json_result, split_json_result, + title, x_label, y_label, ): @@ -125,7 +126,7 @@ def verify_vega( assert "" in split_json_result[0]["content"] def _assert_templates_equal( - html_template, filled_template, split_template, x_label, y_label + html_template, filled_template, split_template, title, x_label, y_label ): # besides split anchors, json and split json should be equal paths = [["data", "values"], ["encoding", "color"]] @@ -133,8 +134,19 @@ def _assert_templates_equal( tmp2 = deepcopy(filled_template) tmp3 = json.loads( split_template[:] + .replace("", title) .replace("", x_label) .replace("", y_label) + .replace( + '""', + json.dumps( + { + "name": "grid", + "select": "interval", + "bind": "scales", + } + ), + ) ) for path in paths: dpath.set(tmp1, path, {}) @@ -147,6 +159,7 @@ def _assert_templates_equal( html_result, json_result[0]["content"], split_json_result[0]["content"], + title, x_label, y_label, ) @@ -235,15 +248,16 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ) verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_data) - for plot, x_label, y_label in [ - ("linear.json", "x", "y"), - ("confusion.json", "predicted", "actual"), + for plot, title, x_label, y_label in [ + ("linear.json", "linear", "x", "y"), + ("confusion.json", "confusion matrix", "predicted", "actual"), ]: verify_vega( "workspace", html_result[plot], json_data[plot], split_json_data[plot], + title, x_label, y_label, ) @@ -264,15 +278,16 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ 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, x_label, y_label in [ - ("linear.json", "x", "y"), - ("confusion.json", "predicted", "actual"), + for plot, title, x_label, y_label in [ + ("linear.json", "linear", "x", "y"), + ("confusion.json", "confusion matrix", "predicted", "actual"), ]: verify_vega( ["HEAD", "workspace"], html_result[plot], json_data[plot], split_json_data[plot], + title, x_label, y_label, ) @@ -340,15 +355,16 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ }, ) - for plot, x_label, y_label in [ - ("../linear.json", "x", "y"), - ("../confusion.json", "predicted", "actual"), + for plot, title, x_label, y_label in [ + ("../linear.json", "linear", "x", "y"), + ("../confusion.json", "confusion matrix", "predicted", "actual"), ]: verify_vega( ["HEAD", "workspace"], html_result[plot], json_data[plot], split_json_data[plot], + title, x_label, y_label, ) From 87c470ba43896a6dd621bdec5fb7dc6fbb671a71 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 5 Oct 2023 21:00:07 +1100 Subject: [PATCH 08/14] switch anchor_revs to revs_with_datapoints --- dvc/render/match.py | 6 +++--- tests/unit/render/test_match.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dvc/render/match.py b/dvc/render/match.py index 0985ced4e6..0a861cb4fd 100644 --- a/dvc/render/match.py +++ b/dvc/render/match.py @@ -101,8 +101,6 @@ def match_defs_renderers( # noqa: C901, PLR0912 else: renderer_cls = VegaRenderer renderer_id = plot_id - if rev not in revs: - revs.append(rev) converter = _get_converter(renderer_cls, inner_id, props, definitions_data) @@ -112,6 +110,8 @@ def match_defs_renderers( # noqa: C901, PLR0912 try: dps, rev_props = converter.flat_datapoints(rev) + if dps and rev not in revs: + revs.append(rev) except Exception as e: # noqa: BLE001 logger.warning("In %r, %s", rev, str(e).lower()) def_errors[rev] = e @@ -125,7 +125,7 @@ def match_defs_renderers( # noqa: C901, PLR0912 first_props["title"] = renderer_id if revs: - first_props["anchor_revs"] = revs + first_props["revs_with_datapoints"] = revs if renderer_cls is not None: renderer = renderer_cls(plot_datapoints, renderer_id, **first_props) diff --git a/tests/unit/render/test_match.py b/tests/unit/render/test_match.py index c590f6c836..18b6c8b8b6 100644 --- a/tests/unit/render/test_match.py +++ b/tests/unit/render/test_match.py @@ -173,7 +173,7 @@ def test_match_renderers(M): ] assert renderer.properties == { "anchors_y_definitions": [{"filename": "file.json", "field": "y"}], - "anchor_revs": ["v1", "revision_with_no_data"], + "revs_with_datapoints": ["v1"], "title": "plot_id_1", "x": "x", "y": "y", From 9c8e08fe60a29fc37c0298e39fddc4bc65f4ca9b Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 6 Oct 2023 19:44:26 +1100 Subject: [PATCH 09/14] accomodate height and width anchors --- tests/integration/plots/test_plots.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 18d27eafdc..630278eced 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -134,6 +134,10 @@ def _assert_templates_equal( tmp2 = deepcopy(filled_template) tmp3 = json.loads( split_template[:] + .replace('""', "300") + .replace('""', "300") + .replace('""', "300") + .replace('""', "300") .replace("", title) .replace("", x_label) .replace("", y_label) From da81fc45696c11839807208d4b671673953d94d7 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 9 Nov 2023 09:48:25 +1100 Subject: [PATCH 10/14] drop terrible idea of holding all data as strings --- dvc/commands/plots.py | 2 +- dvc/render/convert.py | 2 +- tests/integration/plots/test_plots.py | 26 ++++++++++++++------------ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/dvc/commands/plots.py b/dvc/commands/plots.py index a53be3f35e..4c22ea2fe7 100644 --- a/dvc/commands/plots.py +++ b/dvc/commands/plots.py @@ -135,7 +135,7 @@ def run(self) -> int: # noqa: C901, PLR0911, PLR0912 if self.args.show_vega: renderer = first(filter(lambda r: r.TYPE == "vega", renderers)) if renderer: - ui.write_json(renderer.get_filled_template(as_string=False)) + ui.write_json(renderer.get_filled_template()) return 0 output_file: Path = (Path.cwd() / out).resolve() / "index.html" diff --git a/dvc/render/convert.py b/dvc/render/convert.py index b5466839b3..bfbc6fa40e 100644 --- a/dvc/render/convert.py +++ b/dvc/render/convert.py @@ -26,7 +26,7 @@ def to_json(renderer, split: bool = False) -> List[Dict]: if split: content, split_content = renderer.get_partial_filled_template() else: - content = renderer.get_filled_template(as_string=False) + content = renderer.get_filled_template() split_content = {} return [ diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 630278eced..20053b0683 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -114,16 +114,20 @@ def verify_vega( assert j[0]["type"] == "vega" assert set(j[0]["revisions"]) == set(versions) - assert json_result[0]["content"]["data"]["values"] == json.loads( - split_json_result[0]["anchor_definitions"][""] + assert ( + json_result[0]["content"]["data"]["values"] + == split_json_result[0]["anchor_definitions"][""] ) + assert set(versions) == set(json_result[0]["revisions"]) assert json_result[0]["content"]["data"]["values"] assert html_result["data"]["values"] - assert "" in split_json_result[0]["content"] - assert "" in split_json_result[0]["content"] - assert "" in split_json_result[0]["content"] + + content_str = json.dumps(split_json_result[0]["content"]) + assert "" in content_str + assert "" in content_str + assert "" in content_str def _assert_templates_equal( html_template, filled_template, split_template, title, x_label, y_label @@ -133,7 +137,7 @@ def _assert_templates_equal( tmp1 = deepcopy(html_template) tmp2 = deepcopy(filled_template) tmp3 = json.loads( - split_template[:] + json.dumps(split_template) .replace('""', "300") .replace('""', "300") .replace('""', "300") @@ -486,11 +490,9 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): assert html_result["linear_train_vs_test"]["data"]["values"] == ble assert ( - json.loads( - split_json_result["data"]["linear_train_vs_test"][0]["anchor_definitions"][ - "" - ] - ) + split_json_result["data"]["linear_train_vs_test"][0]["anchor_definitions"][ + "" + ] == ble ) @@ -501,7 +503,7 @@ def test_repo_with_dvclive_plots(tmp_dir, capsys, repo_with_dvclive_plots): for s in ("show", "diff"): _, json_result, split_json_result = call(capsys, subcommand=s) - expected_result = { + expected_result: Dict[str, Dict[str, list[str]]] = { "data": { "dvclive/plots/metrics/metric.tsv": [], }, From 5f6eac080342ae246bd7f27f8f7d3b3dae024835 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 27 Nov 2023 14:13:14 +1100 Subject: [PATCH 11/14] use constants in tests --- tests/integration/plots/test_plots.py | 30 ++-- tests/unit/render/test_convert.py | 10 +- tests/unit/render/test_image_converter.py | 8 +- tests/unit/render/test_match.py | 21 ++- tests/unit/render/test_vega_converter.py | 189 +++++++++++----------- 5 files changed, 131 insertions(+), 127 deletions(-) diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 20053b0683..4232f89ab7 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -11,7 +11,7 @@ from funcy import first from dvc.cli import main -from dvc.render import REVISION +from dvc.render import ANCHOR_DEFINITIONS, FILENAME, REVISION JSON_OUT = "vis_data" @@ -116,7 +116,7 @@ def verify_vega( assert ( json_result[0]["content"]["data"]["values"] - == split_json_result[0]["anchor_definitions"][""] + == split_json_result[0][ANCHOR_DEFINITIONS][""] ) assert set(versions) == set(json_result[0]["revisions"]) @@ -138,10 +138,8 @@ def _assert_templates_equal( tmp2 = deepcopy(filled_template) tmp3 = json.loads( json.dumps(split_template) - .replace('""', "300") .replace('""', "300") .replace('""', "300") - .replace('""', "300") .replace("", title) .replace("", x_label) .replace("", y_label) @@ -231,7 +229,7 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( linear_v1, { - "rev": "workspace", + REVISION: "workspace", }, ) assert html_result["linear.json"]["data"]["values"] == _update_datapoints( @@ -245,13 +243,13 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( confusion_v1, { - "rev": "workspace", + REVISION: "workspace", }, ) assert html_result["confusion.json"]["data"]["values"] == _update_datapoints( confusion_v1, { - "rev": "workspace", + REVISION: "workspace", }, ) verify_image(tmp_dir, "workspace", "image.png", image_v1, html_path, json_data) @@ -319,12 +317,12 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( linear_v2, { - "rev": "workspace", + REVISION: "workspace", }, ) + _update_datapoints( linear_v1, { - "rev": "HEAD", + REVISION: "HEAD", }, ) assert html_result["../linear.json"]["data"]["values"] == _update_datapoints( @@ -343,12 +341,12 @@ def test_repo_with_plots(tmp_dir, scm, dvc, capsys, run_copy_metrics, repo_with_ ] == _update_datapoints( confusion_v2, { - "rev": "workspace", + REVISION: "workspace", }, ) + _update_datapoints( confusion_v1, { - "rev": "HEAD", + REVISION: "HEAD", }, ) assert html_result["../confusion.json"]["data"]["values"] == _update_datapoints( @@ -477,20 +475,20 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): ble = _update_datapoints( plots["data"]["linear_train.json"], { - "rev": "workspace", - "filename": "linear_train.json", + REVISION: "workspace", + FILENAME: "linear_train.json", }, ) + _update_datapoints( plots["data"]["linear_test.json"], { - "rev": "workspace", - "filename": "linear_test.json", + REVISION: "workspace", + FILENAME: "linear_test.json", }, ) assert html_result["linear_train_vs_test"]["data"]["values"] == ble assert ( - split_json_result["data"]["linear_train_vs_test"][0]["anchor_definitions"][ + split_json_result["data"]["linear_train_vs_test"][0][ANCHOR_DEFINITIONS][ "" ] == ble diff --git a/tests/unit/render/test_convert.py b/tests/unit/render/test_convert.py index 204314e95d..68db9d4db5 100644 --- a/tests/unit/render/test_convert.py +++ b/tests/unit/render/test_convert.py @@ -2,7 +2,7 @@ import pytest -from dvc.render import ANCHOR_DEFINITIONS, REVISION, REVISIONS, SRC, TYPE_KEY +from dvc.render import ANCHOR_DEFINITIONS, FILENAME, REVISION, REVISIONS, SRC, TYPE_KEY from dvc.render.convert import to_json @@ -42,14 +42,14 @@ def test_to_json_vega_split(mocker): { "x": 1, "y": 2, - "rev": "foo", - "filename": "foo.json", + REVISION: "foo", + FILENAME: "foo.json", }, { "x": 2, "y": 1, - "rev": "bar", - "filename": "foo.json", + REVISION: "bar", + FILENAME: "foo.json", }, ], } diff --git a/tests/unit/render/test_image_converter.py b/tests/unit/render/test_image_converter.py index 3081203078..20f317dbd4 100644 --- a/tests/unit/render/test_image_converter.py +++ b/tests/unit/render/test_image_converter.py @@ -1,4 +1,4 @@ -from dvc.render import REVISION, SRC +from dvc.render import FILENAME, REVISION, SRC from dvc.render.converter.image import ImageConverter @@ -9,7 +9,7 @@ def test_image_converter_no_out(): assert datapoints[0] == { REVISION: "r", - "filename": "image.png", + FILENAME: "image.png", SRC: converter._encode_image(b"content"), } @@ -22,7 +22,7 @@ def test_image_converter_with_out(tmp_dir): assert datapoints[0] == { REVISION: "r", - "filename": "image.png", + FILENAME: "image.png", SRC: str(tmp_dir / "foo" / "r_image.png"), } @@ -38,7 +38,7 @@ def test_image_converter_with_slash_in_revision(tmp_dir): assert datapoints[0] == { REVISION: "feature/r", - "filename": "image.png", + FILENAME: "image.png", SRC: str(tmp_dir / "foo" / "feature_r_image.png"), } diff --git a/tests/unit/render/test_match.py b/tests/unit/render/test_match.py index 18b6c8b8b6..c5f417a32b 100644 --- a/tests/unit/render/test_match.py +++ b/tests/unit/render/test_match.py @@ -1,8 +1,13 @@ import pytest from funcy import set_in +from dvc.render import FIELD, FILENAME, REVISION from dvc.render.converter.vega import VegaConverter -from dvc.render.match import PlotsData, _squash_plots_properties, match_defs_renderers +from dvc.render.match import ( + PlotsData, + _squash_plots_properties, + match_defs_renderers, +) @pytest.mark.parametrize( @@ -157,22 +162,22 @@ def test_match_renderers(M): renderer = renderer_with_errors[0] assert renderer.datapoints == [ { - "rev": "v1", - "filename": "file.json", - "field": "y", + REVISION: "v1", + FILENAME: "file.json", + FIELD: "y", "x": 1, "y": 1, }, { - "rev": "v1", - "filename": "file.json", - "field": "y", + REVISION: "v1", + FILENAME: "file.json", + FIELD: "y", "x": 2, "y": 2, }, ] assert renderer.properties == { - "anchors_y_definitions": [{"filename": "file.json", "field": "y"}], + "anchors_y_definitions": [{FILENAME: "file.json", FIELD: "y"}], "revs_with_datapoints": ["v1"], "title": "plot_id_1", "x": "x", diff --git a/tests/unit/render/test_vega_converter.py b/tests/unit/render/test_vega_converter.py index 4710dbc140..4d5ca0434c 100644 --- a/tests/unit/render/test_vega_converter.py +++ b/tests/unit/render/test_vega_converter.py @@ -3,6 +3,7 @@ import pytest from dvc.exceptions import DvcException +from dvc.render import FIELD, FILENAME, REVISION from dvc.render.converter.vega import FieldNotFoundError, VegaConverter, _lists @@ -34,20 +35,20 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "step": 0, - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", }, { "v": 2, "step": 1, - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", }, ], { - "anchors_y_definitions": [{"filename": "f", "field": "v"}], + "anchors_y_definitions": [{FILENAME: "f", FIELD: "v"}], "x": "step", "y": "v", "x_label": "step", @@ -62,20 +63,20 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, { "v": 2, "v2": 0.2, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, ], { - "anchors_y_definitions": [{"filename": "f", "field": "v2"}], + "anchors_y_definitions": [{FILENAME: "f", FIELD: "v2"}], "x": "v", "y": "v2", "x_label": "v", @@ -102,20 +103,20 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, { "v": 2, "v2": 0.2, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, ], { - "anchors_y_definitions": [{"filename": "f", "field": "v2"}], + "anchors_y_definitions": [{FILENAME: "f", FIELD: "v2"}], "x": "v", "y": "v2", "x_label": "x", @@ -128,36 +129,36 @@ def test_finding_lists(dictionary, expected_result): {"y": {"f": ["v", "v2"]}}, [ { - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", "dvc_inferred_y_value": 1, "v": 1, "v2": 0.1, "step": 0, }, { - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", "dvc_inferred_y_value": 2, "v": 2, "v2": 0.2, "step": 1, }, { - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", "dvc_inferred_y_value": 0.1, "v2": 0.1, "v": 1, "step": 0, }, { - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", "v": 2, "v2": 0.2, "dvc_inferred_y_value": 0.2, @@ -166,8 +167,8 @@ def test_finding_lists(dictionary, expected_result): ], { "anchors_y_definitions": [ - {"filename": "f", "field": "v"}, - {"filename": "f", "field": "v2"}, + {FILENAME: "f", FIELD: "v"}, + {FILENAME: "f", FIELD: "v2"}, ], "x": "step", "y": "dvc_inferred_y_value", @@ -190,42 +191,42 @@ def test_finding_lists(dictionary, expected_result): "z": 3, "v": 1, "step": 0, - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", }, { "dvc_inferred_y_value": 2, "z": 4, "step": 1, "v": 2, - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", }, { "dvc_inferred_y_value": 3, "v": 1, "z": 3, "step": 0, - "rev": "r", - "filename": "f", - "field": "z", + REVISION: "r", + FILENAME: "f", + FIELD: "z", }, { "dvc_inferred_y_value": 4, "v": 2, "z": 4, "step": 1, - "rev": "r", - "filename": "f", - "field": "z", + REVISION: "r", + FILENAME: "f", + FIELD: "z", }, ], { "anchors_y_definitions": [ - {"filename": "f", "field": "v"}, - {"filename": "f", "field": "z"}, + {FILENAME: "f", FIELD: "v"}, + {FILENAME: "f", FIELD: "z"}, ], "x": "step", "y": "dvc_inferred_y_value", @@ -244,29 +245,29 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, { "v": 2, "v2": 0.2, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, { "v": 3, "v2": 0.3, - "rev": "r", - "filename": "f2", - "field": "v2", + REVISION: "r", + FILENAME: "f2", + FIELD: "v2", }, ], { "anchors_y_definitions": [ - {"filename": "f", "field": "v2"}, - {"filename": "f2", "field": "v2"}, + {FILENAME: "f", FIELD: "v2"}, + {FILENAME: "f2", FIELD: "v2"}, ], "x": "v", "y": "v2", @@ -284,42 +285,42 @@ def test_finding_lists(dictionary, expected_result): "v": 1, "v2": 0.1, "step": 0, - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", }, { "dvc_inferred_y_value": 2, "v": 2, "v2": 0.2, "step": 1, - "rev": "r", - "filename": "f", - "field": "v", + REVISION: "r", + FILENAME: "f", + FIELD: "v", }, { "dvc_inferred_y_value": 0.1, "v": 1, "v2": 0.1, "step": 0, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, { "dvc_inferred_y_value": 0.2, "v": 2, "v2": 0.2, "step": 1, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, ], { "anchors_y_definitions": [ - {"filename": "f", "field": "v"}, - {"filename": "f", "field": "v2"}, + {FILENAME: "f", FIELD: "v"}, + {FILENAME: "f", FIELD: "v2"}, ], "x": "step", "y": "dvc_inferred_y_value", @@ -340,33 +341,33 @@ def test_finding_lists(dictionary, expected_result): "v": 1, "v2": 0.1, "v3": 0.01, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, { "dvc_inferred_y_value": 0.01, "v": 1, "v2": 0.1, "v3": 0.01, - "rev": "r", - "filename": "f", - "field": "v3", + REVISION: "r", + FILENAME: "f", + FIELD: "v3", }, { "dvc_inferred_y_value": 0.1, "v": 1, "v2": 0.1, - "rev": "r", - "filename": "f2", - "field": "v2", + REVISION: "r", + FILENAME: "f2", + FIELD: "v2", }, ], { "anchors_y_definitions": [ - {"filename": "f", "field": "v2"}, - {"filename": "f", "field": "v3"}, - {"filename": "f2", "field": "v2"}, + {FILENAME: "f", FIELD: "v2"}, + {FILENAME: "f", FIELD: "v3"}, + {FILENAME: "f2", FIELD: "v2"}, ], "x": "v", "y": "dvc_inferred_y_value", @@ -385,22 +386,22 @@ def test_finding_lists(dictionary, expected_result): { "v": 1, "v2": 0.1, - "rev": "r", - "filename": "f", - "field": "v2", + REVISION: "r", + FILENAME: "f", + FIELD: "v2", }, { "v": 1, "v2": 0.1, - "rev": "r", - "filename": "f2", - "field": "v2", + REVISION: "r", + FILENAME: "f2", + FIELD: "v2", }, ], { "anchors_y_definitions": [ - {"filename": "f", "field": "v2"}, - {"filename": "f2", "field": "v2"}, + {FILENAME: "f", FIELD: "v2"}, + {FILENAME: "f2", FIELD: "v2"}, ], "x": "v", "y": "v2", From 90134831a99e3c3bb05e481ef2b1eb137cac83fe Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Wed, 29 Nov 2023 08:02:03 +1100 Subject: [PATCH 12/14] fix plot having multiple x fields --- dvc/render/converter/vega.py | 9 +- tests/unit/render/test_vega_converter.py | 101 +++++++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/dvc/render/converter/vega.py b/dvc/render/converter/vega.py index 6838dcdb49..e0fc620a1c 100644 --- a/dvc/render/converter/vega.py +++ b/dvc/render/converter/vega.py @@ -204,11 +204,13 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 ) else: x_file, x_field = xs[0] - props_update["x"] = x_field + + num_xs = len(xs) + unify_x_fields = num_xs > 1 and len({x[1] for x in xs}) > 1 + props_update["x"] = "dvc_inferred_x_value" if unify_x_fields else x_field ys = list(_get_ys(properties, file2datapoints)) - num_xs = len(xs) num_ys = len(ys) if num_xs > 1 and num_xs != num_ys: raise DvcException( @@ -264,8 +266,9 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 try: _update_from_field( datapoints, - field=x_field, + field="dvc_inferred_x_value" if unify_x_fields else x_field, source_datapoints=x_datapoints, + source_field=x_field, ) except IndexError: raise DvcException( # noqa: B904 diff --git a/tests/unit/render/test_vega_converter.py b/tests/unit/render/test_vega_converter.py index 4d5ca0434c..3bdc65a66a 100644 --- a/tests/unit/render/test_vega_converter.py +++ b/tests/unit/render/test_vega_converter.py @@ -410,6 +410,107 @@ def test_finding_lists(dictionary, expected_result): }, id="multi_file_y_same_prefix", ), + pytest.param( + { + "f": {"metric": [{"x1": 1, "v": 0.1}]}, + "f2": {"metric": [{"x2": 100, "v": 0.1}]}, + }, + {"y": {"f": ["v"], "f2": ["v"]}, "x": {"f": "x1", "f2": "x2"}}, + [ + { + "x1": 1, + "v": 0.1, + "dvc_inferred_x_value": 1, + REVISION: "r", + FILENAME: "f", + FIELD: "v", + }, + { + "x2": 100, + "v": 0.1, + "dvc_inferred_x_value": 100, + REVISION: "r", + FILENAME: "f2", + FIELD: "v", + }, + ], + { + "anchors_y_definitions": [ + {FILENAME: "f", FIELD: "v"}, + {FILENAME: "f2", FIELD: "v"}, + ], + "x": "dvc_inferred_x_value", + "y": "v", + "x_label": "x", + "y_label": "v", + }, + id="multiple_x_fields", + ), + pytest.param( + { + "f": { + "metric": [ + {"v": 1, "v2": 0.1, "x1": 100}, + {"v": 2, "v2": 0.2, "x1": 1000}, + ] + }, + "f2": {"metric": [{"x2": -2}, {"x2": -4}]}, + }, + {"y": ["v", "v2"], "x": {"f": "x1", "f2": "x2"}}, + [ + { + "dvc_inferred_x_value": 100, + "dvc_inferred_y_value": 1, + "v": 1, + "v2": 0.1, + "x1": 100, + REVISION: "r", + FILENAME: "f", + FIELD: "v", + }, + { + "dvc_inferred_x_value": 1000, + "dvc_inferred_y_value": 2, + "v": 2, + "v2": 0.2, + "x1": 1000, + REVISION: "r", + FILENAME: "f", + FIELD: "v", + }, + { + "dvc_inferred_x_value": -2, + "dvc_inferred_y_value": 0.1, + "v": 1, + "v2": 0.1, + "x1": 100, + REVISION: "r", + FILENAME: "f", + FIELD: "v2", + }, + { + "dvc_inferred_x_value": -4, + "dvc_inferred_y_value": 0.2, + "v": 2, + "v2": 0.2, + "x1": 1000, + REVISION: "r", + FILENAME: "f", + FIELD: "v2", + }, + ], + { + "anchors_y_definitions": [ + {FILENAME: "f", FIELD: "v"}, + {FILENAME: "f", FIELD: "v2"}, + ], + "x": "dvc_inferred_x_value", + "y": "dvc_inferred_y_value", + "x_label": "x", + "y_label": "y", + }, + id="y_list_x_dict", + ), ], ) def test_convert( From 1f0f869fb42bc978099590a70748b36e590975f3 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Wed, 29 Nov 2023 08:07:42 +1100 Subject: [PATCH 13/14] move inferred properties off vega converter class --- dvc/render/converter/vega.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/dvc/render/converter/vega.py b/dvc/render/converter/vega.py index e0fc620a1c..b31a849891 100644 --- a/dvc/render/converter/vega.py +++ b/dvc/render/converter/vega.py @@ -112,7 +112,6 @@ def __init__( ): super().__init__(plot_id, data, properties) self.plot_id = plot_id - self.inferred_properties: Dict = {} def _infer_y_from_data(self): if self.plot_id in self.data: @@ -120,34 +119,38 @@ def _infer_y_from_data(self): if all(isinstance(item, dict) for item in lst): datapoint = first(lst) field = last(datapoint.keys()) - self.inferred_properties["y"] = {self.plot_id: field} - break + return {self.plot_id: field} + return None def _infer_x_y(self): x = self.properties.get("x", None) y = self.properties.get("y", None) + inferred_properties: Dict = {} + # Infer x. if isinstance(x, str): - self.inferred_properties["x"] = {} + inferred_properties["x"] = {} # If multiple y files, duplicate x for each file. if isinstance(y, dict): for file, fields in y.items(): # Duplicate x for each y. if isinstance(fields, list): - self.inferred_properties["x"][file] = [x] * len(fields) + inferred_properties["x"][file] = [x] * len(fields) else: - self.inferred_properties["x"][file] = x + inferred_properties["x"][file] = x # Otherwise use plot ID as file. else: - self.inferred_properties["x"][self.plot_id] = x + inferred_properties["x"][self.plot_id] = x # Infer y. if y is None: - self._infer_y_from_data() + inferred_properties["y"] = self._infer_y_from_data() # If y files not provided, use plot ID as file. elif not isinstance(y, dict): - self.inferred_properties["y"] = {self.plot_id: y} + inferred_properties["y"] = {self.plot_id: y} + + return inferred_properties def _find_datapoints(self): result = {} @@ -303,10 +306,10 @@ def convert( generated datapoints and updated properties. `x`, `y` values and labels are inferred and always provided. """ - self._infer_x_y() + inferred_properties = self._infer_x_y() datapoints = self._find_datapoints() - properties = {**self.properties, **self.inferred_properties} + properties = {**self.properties, **inferred_properties} properties["y_label"] = self.infer_y_label(properties) properties["x_label"] = self.infer_x_label(properties) From 984860b44d34d850b13cf154171e278dbaef943f Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 4 Dec 2023 15:57:30 +1100 Subject: [PATCH 14/14] improve variable name --- dvc/render/converter/vega.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dvc/render/converter/vega.py b/dvc/render/converter/vega.py index b31a849891..b5cb5a3efd 100644 --- a/dvc/render/converter/vega.py +++ b/dvc/render/converter/vega.py @@ -209,8 +209,8 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 x_file, x_field = xs[0] num_xs = len(xs) - unify_x_fields = num_xs > 1 and len({x[1] for x in xs}) > 1 - props_update["x"] = "dvc_inferred_x_value" if unify_x_fields else x_field + multiple_x_fields = num_xs > 1 and len({x[1] for x in xs}) > 1 + props_update["x"] = "dvc_inferred_x_value" if multiple_x_fields else x_field ys = list(_get_ys(properties, file2datapoints)) @@ -269,7 +269,7 @@ def flat_datapoints(self, revision): # noqa: C901, PLR0912 try: _update_from_field( datapoints, - field="dvc_inferred_x_value" if unify_x_fields else x_field, + field="dvc_inferred_x_value" if multiple_x_fields else x_field, source_datapoints=x_datapoints, source_field=x_field, )