diff --git a/dvc/render/convert.py b/dvc/render/convert.py index c90fd0426..492459af7 100644 --- a/dvc/render/convert.py +++ b/dvc/render/convert.py @@ -14,14 +14,14 @@ from dvc.render.converter.vega import VegaConverter def _get_converter( - renderer_class, props + renderer_class, renderer_id, props, data ) -> Union[VegaConverter, ImageConverter]: from dvc_render import ImageRenderer, VegaRenderer if renderer_class.TYPE == VegaRenderer.TYPE: - return VegaConverter(props) + return VegaConverter(renderer_id, data, props) if renderer_class.TYPE == ImageRenderer.TYPE: - return ImageConverter(props) + return ImageConverter(renderer_id, data, props) raise ValueError(f"Invalid renderer class {renderer_class}") diff --git a/dvc/render/converter/__init__.py b/dvc/render/converter/__init__.py index 4685b2f75..dd7018b5a 100644 --- a/dvc/render/converter/__init__.py +++ b/dvc/render/converter/__init__.py @@ -1,9 +1,19 @@ -from typing import Dict, Optional +from typing import Any, Dict, List, Optional, Tuple class Converter: - def __init__(self, plot_properties: Optional[Dict] = None): - self.plot_properties = plot_properties or {} + def __init__( + self, + plot_id: str, + data: Dict[str, Any] = None, + properties: Optional[Dict] = None, + ): + self.plot_id = plot_id + self.properties = properties or {} + self.data = data or {} - def convert(self, data, revision: str, filename: str, **kwargs): + def convert(self) -> Tuple[List[Tuple[str, str, Any]], Dict]: + raise NotImplementedError + + def flat_datapoints(self, revision: str) -> Tuple[List[Dict], Dict]: raise NotImplementedError diff --git a/dvc/render/converter/image.py b/dvc/render/converter/image.py index 8d3a54581..fba390b30 100644 --- a/dvc/render/converter/image.py +++ b/dvc/render/converter/image.py @@ -1,6 +1,6 @@ import base64 import os -from typing import TYPE_CHECKING, Dict, List, Tuple +from typing import TYPE_CHECKING, Any, Dict, List, Tuple from dvc.render import FILENAME_FIELD, REVISION_FIELD, SRC_FIELD @@ -34,25 +34,33 @@ class ImageConverter(Converter): base64_str = base64.b64encode(image_data).decode() return f"data:image;base64,{base64_str}" - def convert( - self, data, revision: str, filename: str, **kwargs - ) -> Tuple[List[Dict], Dict]: + def convert(self) -> Tuple[List[Tuple[str, str, Any]], Dict]: + datas = [] + for filename, image_data in self.data.items(): + datas.append((filename, "", image_data)) + return datas, self.properties + + def flat_datapoints(self, revision: str) -> Tuple[List[Dict], Dict]: """ Convert the DVC Plots content to DVC Render datapoints. Return both generated datapoints and updated properties. """ - path = self.plot_properties.get("out") - if path: - if not os.path.isdir(path): - os.makedirs(path, exist_ok=True) - src = self._write_image( - os.path.abspath(path), revision, filename, data - ) - else: - src = self._encode_image(data) - datapoint = { - REVISION_FIELD: revision, - FILENAME_FIELD: filename, - SRC_FIELD: src, - } - return [datapoint], self.plot_properties + path = self.properties.get("out") + datapoints = [] + datas, properties = self.convert() + for filename, _, image_data in datas: + if path: + if not os.path.isdir(path): + os.makedirs(path, exist_ok=True) + src = self._write_image( + os.path.abspath(path), revision, filename, image_data + ) + else: + src = self._encode_image(image_data) + datapoint = { + REVISION_FIELD: revision, + FILENAME_FIELD: filename, + SRC_FIELD: src, + } + datapoints.append(datapoint) + return datapoints, properties diff --git a/dvc/render/converter/vega.py b/dvc/render/converter/vega.py index 5568ff665..e723253f5 100644 --- a/dvc/render/converter/vega.py +++ b/dvc/render/converter/vega.py @@ -1,90 +1,101 @@ from copy import deepcopy -from functools import partial -from typing import Dict, Iterable, List, Optional, Set, Union +from typing import Any, Dict, Iterable, List, Tuple, Union -from funcy import first, project +from funcy import first, last from dvc.exceptions import DvcException -from dvc.render import ( - FILENAME_FIELD, - INDEX_FIELD, - REVISION_FIELD, - VERSION_FIELD, -) +from dvc.render import FILENAME_FIELD, INDEX_FIELD, VERSION_FIELD from . import Converter -class FieldsNotFoundError(DvcException): - def __init__(self, expected_fields, found_fields): - expected_str = ", ".join(expected_fields) +class FieldNotFoundError(DvcException): + def __init__(self, expected_field, found_fields): found_str = ", ".join(found_fields) super().__init__( - f"Could not find all provided fields ('{expected_str}') " + f"Could not find provided field ('{expected_field}') " f"in data fields ('{found_str}')." ) -class PlotDataStructureError(DvcException): - def __init__(self): - super().__init__( - "Plot data extraction failed. Please see " - "https://man.dvc.org/plots for supported data formats." - ) +def _lists(blob: Union[Dict, List]) -> Iterable[List]: + if isinstance(blob, list): + yield blob + else: + for _, value in blob.items(): + if isinstance(value, dict): + yield from _lists(value) + elif isinstance(value, list): + yield value -def _filter_fields( - datapoints: List[Dict], fields: Set, **kwargs -) -> List[Dict]: - if not fields: - return datapoints - assert isinstance(fields, set) - - new_data = [] - for data_point in datapoints: - keys = set(data_point.keys()) - if not fields <= keys: - raise FieldsNotFoundError(fields, keys) - - new_data.append(project(data_point, fields)) - - return new_data +def _file_field(*args): + for axis_def in args: + if axis_def is not None: + for file, val in axis_def.items(): + if isinstance(val, str): + yield file, val + elif isinstance(val, list): + for field in val: + yield file, field -def _lists(dictionary: Dict): - for _, value in dictionary.items(): - if isinstance(value, dict): - yield from _lists(value) - elif isinstance(value, list): - yield value +def _find( + filename: str, + field: str, + data_series: List[Tuple[str, str, Any]], +): + for data_file, data_field, data in data_series: + if data_file == filename and data_field == field: + return data_file, data_field, data + return None -def _find_first_list( - data: Union[Dict, List], fields: Set, **kwargs -) -> List[Dict]: - fields = fields or set() - - if not isinstance(data, dict): - return data - - for lst in _lists(data): - if ( - all(isinstance(dp, dict) for dp in lst) - # if fields is empty, it will match any set - and set(first(lst).keys()) & fields == fields - ): - return lst - - raise PlotDataStructureError() +def _verify_field(file2datapoints: Dict[str, List], filename: str, field: str): + if filename in file2datapoints: + datapoint = first(file2datapoints[filename]) + if field not in datapoint: + raise FieldNotFoundError(field, datapoint.keys()) + return -def _append_index(datapoints: List[Dict], **kwargs) -> List[Dict]: - if INDEX_FIELD in first(datapoints).keys(): - return datapoints +def _get_x(properties: Dict, file2datapoints: Dict[str, List[Dict]]): + x = properties.get("x", None) + if x is not None and isinstance(x, dict): + filename, field = first(_file_field(x)) + _verify_field(file2datapoints, filename, field) + return filename, field + return None - for index, data_point in enumerate(datapoints): - data_point[INDEX_FIELD] = index - return datapoints + +def _get_ys(properties, file2datapoints: Dict[str, List[Dict]]): + y = properties.get("y", None) + if y is not None: + for filename, field in _file_field(y): + _verify_field(file2datapoints, filename, field) + yield filename, field + + +def _is_datapoints(lst: List[Dict]): + """ + check if dict keys match, datapoints with different keys mgiht lead + to unexpected behavior + """ + + return all(isinstance(item, dict) for item in lst) and set( + first(lst).keys() + ) == {key for keys in lst for key in keys} + + +def get_datapoints(file_content: Dict): + result: List[Dict[str, Any]] = [] + for lst in _lists(file_content): + if _is_datapoints(lst): + for index, datapoint in enumerate(lst): + if len(result) <= index: + result.append({}) + result[index].update(datapoint) + return result class VegaConverter(Converter): @@ -95,228 +106,213 @@ class VegaConverter(Converter): ('x', 'y') it will attempt to fill in the blanks. """ - def __init__(self, plot_properties: Optional[Dict] = None): - super().__init__(plot_properties) + def __init__( + self, plot_id: str, data: Dict = None, properties: Dict = None + ): + super().__init__(plot_id, data, properties) + self.plot_id = plot_id self.inferred_properties: Dict = {} - self.steps = [] + # TODO we should be handling that in `convert`, + # to avoid stateful `self.inferred_properties` + self._infer_x_y() - self._infer_x() - self._infer_fields() + def _infer_y_from_data(self): + if self.plot_id in self.data: + for lst in _lists(self.data[self.plot_id]): + 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 - self.steps.append( - ( - "find_data", - partial( - _find_first_list, - fields=self.inferred_properties.get("fields", set()) - - {INDEX_FIELD}, - ), - ) - ) + def _infer_x_y(self): + def _infer_files(from_name, to_name): + from_value = self.properties.get(from_name, None) + to_value = self.properties.get(to_name, None) - if not self.plot_properties.get("x", None): - self.steps.append(("append_index", partial(_append_index))) - - self.steps.append( - ( - "filter_fields", - partial( - _filter_fields, - fields=self.inferred_properties.get("fields", set()), - ), - ) - ) - self.steps.append( - ( - "infer_y", - partial( - self._infer_y, - ), - ) - ) - - self.steps.append( - ( - "generate_y", - partial( - self._generate_y_values, - ), - ) - ) - - def _infer_x(self): - if not self.plot_properties.get("x", None): - self.inferred_properties["x"] = INDEX_FIELD - - def skip_step(self, name: str): - self.steps = [(_name, fn) for _name, fn in self.steps if _name != name] - - def _infer_fields(self): - fields = self.plot_properties.get("fields", set()) - if fields: - fields = { - *fields, - self.plot_properties.get("x", None), - self.plot_properties.get("y", None), - self.inferred_properties.get("x", None), - } - {None} - self.inferred_properties["fields"] = fields - - def _infer_y(self, datapoints: List[Dict], **kwargs): - if "y" not in self.plot_properties: - data_fields = list(first(datapoints)) - skip = ( - REVISION_FIELD, - self.plot_properties.get("x", None) - or self.inferred_properties.get("x"), - FILENAME_FIELD, - VERSION_FIELD, - ) - inferred_y = first( - f for f in reversed(data_fields) if f not in skip - ) - if "y" in self.inferred_properties: - previous_y = self.inferred_properties["y"] - if previous_y != inferred_y: - raise DvcException( - f"Inferred y ('{inferred_y}' value does not match" - f"previously matched one ('f{previous_y}')." - ) + if isinstance(to_value, str): + self.inferred_properties[to_name] = {} + if isinstance(from_value, dict): + for file in from_value.keys(): + self.inferred_properties[to_name][file] = to_value + else: + self.inferred_properties[to_name][self.plot_id] = to_value else: - self.inferred_properties["y"] = inferred_y - return datapoints + self.inferred_properties[to_name] = to_value + + _infer_files("y", "x") + _infer_files("x", "y") + + if self.inferred_properties.get("y", None) is None: + self._infer_y_from_data() + + def _find_datapoints(self): + result = {} + for file, content in self.data.items(): + result[file] = get_datapoints(content) + + return result + + @staticmethod + def infer_y_label(properties): + y_label = properties.get("y_label", None) + if y_label is None: + y = properties.get("y", None) + if isinstance(y, dict): + fields = {field for _, field in _file_field(y)} + if len(fields) == 1: + y_label = first(fields) + else: + y_label = "y" + elif isinstance(y, list): + y_label = "y" + elif isinstance(y, str): + y_label = y + + return y_label + + @staticmethod + def infer_x_label(properties): + x_label = properties.get("x_label", None) + if x_label is None: + x = properties.get("x", None) + if isinstance(x, dict): + fields = {field for _, field in _file_field(x)} + if len(fields) == 1: + x_label = first(fields) + else: + x_label = INDEX_FIELD + return x_label + + def flat_datapoints(self, revision): + + file2datapoints, properties = self.convert() + + props_update = {} + + x = _get_x(properties, file2datapoints) + + # assign "step" if no x provided + if not x: + x_file, x_field = ( + None, + INDEX_FIELD, + ) + else: + x_file, x_field = x + props_update["x"] = x_field + + ys = list(_get_ys(properties, file2datapoints)) + + all_datapoints = [] + all_y_fields = {y_field for _, y_field in ys} + + # override to unified y field name if there are different y fields + if len(all_y_fields) > 1: + props_update["y"] = "dvc_inferred_y_value" + else: + props_update["y"] = first(all_y_fields) + + for y_file, y_field in ys: + datapoints = deepcopy(file2datapoints.get(y_file, [])) + + if props_update.get("y", None) == "dvc_inferred_y_value": + _update_from_field( + datapoints, + field="dvc_inferred_y_value", + source_field=y_field, + ) + + if x_field == INDEX_FIELD and x_file is None: + _update_from_index(datapoints, INDEX_FIELD) + else: + x_datapoints = file2datapoints.get(x_file, []) + try: + _update_from_field( + datapoints, + field=x_field, + source_datapoints=x_datapoints, + ) + except IndexError: + raise DvcException( + f"Cannot join '{x_field}' from '{x_file}' and " + "'{y_field}' from '{y_file}'. " + "They have to have same length." + ) + + _update_all( + datapoints, + update_dict={ + VERSION_FIELD: { + "revision": revision, + FILENAME_FIELD: y_file, + "field": y_field, + } + }, + ) + + all_datapoints.extend(datapoints) + + if not all_datapoints: + return [], {} + + properties = {**properties, **props_update} + + return all_datapoints, properties def convert( self, - data, - revision: str, - filename: str, - skip: List = None, - **kwargs, ): """ Convert the data. Fill necessary fields ('x', 'y') and return both - generated datapoints and updated properties. + generated datapoints and updated properties. If `x` is not provided, + leave it as None, fronteds should handle it. + + NOTE: Studio uses this method. + The only thing studio FE handles is filling `x` and `y`. + `x/y_label` should be filled here. + + Datapoints are not stripped according to config, because users + might be utilizing other fields in their custom plots. """ - if not skip: - skip = [] + datapoints = self._find_datapoints() + properties = {**self.properties, **self.inferred_properties} - processed = deepcopy(data) + properties["y_label"] = self.infer_y_label(properties) + properties["x_label"] = self.infer_x_label(properties) - for step_name, step in self.steps: - if step_name not in skip: - processed = step( # type: ignore - processed, - revision=revision, - filename=filename, - ) + return datapoints, properties - return processed, {**self.plot_properties, **self.inferred_properties} - def _generate_y_values( # noqa: C901 - self, - datapoints: List[Dict], - revision: str, - filename: str, - **kwargs, - ) -> List[Dict]: +def _update_from_field( + target_datapoints: List[Dict], + field: str, + source_datapoints: List[Dict] = None, + source_field: str = None, +): + if source_datapoints is None: + source_datapoints = target_datapoints + if source_field is None: + source_field = field - y_values = self.plot_properties.get( - "y", None - ) or self.inferred_properties.get("y", None) + if len(source_datapoints) != len(target_datapoints): + raise IndexError( + "Source and target datapoints must have the same length" + ) - assert y_values is not None + for index, datapoint in enumerate(target_datapoints): + source_datapoint = source_datapoints[index] + if source_field in source_datapoint: + datapoint[field] = source_datapoint[source_field] - result = [] - properties_update = {} - def _add_version_info(datapoint, version_info): - tmp = datapoint.copy() - tmp[VERSION_FIELD] = version_info - return tmp +def _update_from_index(datapoints: List[Dict], new_field: str): + for index, datapoint in enumerate(datapoints): + datapoint[new_field] = index - def _version_info(revision, filename=None, field=None): - res = {"revision": revision} - if filename is not None: - res["filename"] = filename - if field is not None: - res["field"] = field - return res - def _generate_y(datapoint, field): - tmp = datapoint.copy() - tmp["dvc_inferred_y_value"] = datapoint[field] - tmp = _add_version_info( - tmp, _version_info(revision, filename, field) - ) - if ( - "y_label" not in properties_update - and "y_label" not in self.plot_properties - ): - properties_update["y_label"] = "y" +def _update_all(datapoints: List[Dict], update_dict: Dict): - properties_update["y"] = "dvc_inferred_y_value" - - return tmp - - if isinstance(y_values, str): - for datapoint in datapoints: - result.append( - _add_version_info( - datapoint, _version_info(revision, filename, y_values) - ) - ) - - if isinstance(y_values, list): - for datapoint in datapoints: - for y_val in y_values: - if y_val in datapoint: - result.append(_generate_y(datapoint, y_val)) - - if isinstance(y_values, dict): - - def _to_set(values: Iterable): - result = set() - for val in values: - if isinstance(val, list): - for elem in val: - result.add(elem) - else: - result.add(val) - - return result - - all_fields = _to_set(y_values.values()) - if ( - all([isinstance(field, str) for field in all_fields]) - and len(all_fields) == 1 - ): - # if we use the same field from all files, - # we dont have to generate it - y_field = all_fields.pop() - for datapoint in datapoints: - result.append( - _add_version_info( - datapoint, - _version_info(revision, filename, y_field), - ) - ) - properties_update.update({"y": y_field}) - else: - for def_filename, val in y_values.items(): - if isinstance(val, str): - fields = [val] - if isinstance(val, list): - fields = val - for datapoint in datapoints: - for field in fields: - if field in datapoint and def_filename in filename: - result.append(_generate_y(datapoint, field)) - - self.inferred_properties = { - **self.inferred_properties, - **properties_update, - } - - return result + for datapoint in datapoints: + datapoint.update(update_dict) diff --git a/dvc/render/match.py b/dvc/render/match.py index 96c9275b4..788254138 100644 --- a/dvc/render/match.py +++ b/dvc/render/match.py @@ -42,7 +42,7 @@ class PlotsData: return dict(groups) def get_definition_data(self, target_files, rev): - result = [] + result = {} for file in target_files: file_content = ( self.data.get(rev, {}) @@ -52,7 +52,7 @@ class PlotsData: .get("data", {}) ) if file_content: - result.append((file, file_content)) + result[file] = file_content return result @@ -91,15 +91,14 @@ def match_defs_renderers( renderer_cls = VegaRenderer renderer_id = plot_id - converter = _get_converter(renderer_cls, props) + converter = _get_converter( + renderer_cls, inner_id, props, definitions_data + ) - for filename, plot_data in definitions_data: - dps, final_props = converter.convert( - revision=rev, - filename=filename, - data=plot_data, - ) - plot_datapoints.extend(dps) + dps, rev_props = converter.flat_datapoints(rev) + 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 diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py index 5fc76e4d1..755980ddd 100644 --- a/dvc/repo/plots/__init__.py +++ b/dvc/repo/plots/__init__.py @@ -19,7 +19,7 @@ from typing import ( import dpath.options import dpath.util -from funcy import cached_property, first, project +from funcy import cached_property, distinct, first, project from dvc.exceptions import DvcException from dvc.utils import error_handler, errored_revisions, onerror_collect @@ -314,16 +314,18 @@ def _get_data_targets(definitions: Dict): def infer_data_sources(plot_id, config=None): - def _deduplicate(lst: List): - return list({elem: None for elem in lst}.keys()) - y = config.get("y", None) + if isinstance(y, dict): sources = list(y.keys()) else: sources = [plot_id] - return _deduplicate(source for source in sources) + x = config.get("x", None) + if isinstance(x, dict): + sources.append(first(x.keys())) + + return distinct(source for source in sources) def _matches(targets, config_file, plot_id): diff --git a/dvc/schema.py b/dvc/schema.py index fee993596..b48f61aa0 100644 --- a/dvc/schema.py +++ b/dvc/schema.py @@ -104,7 +104,7 @@ def either_or(primary, fallback, fallback_includes=None): PLOT_DEFINITION = { - Output.PARAM_PLOT_X: str, + Output.PARAM_PLOT_X: Any(str, {str: str}), Output.PARAM_PLOT_Y: Any(str, [str], {str: Any(str, [str])}), Output.PARAM_PLOT_X_LABEL: str, Output.PARAM_PLOT_Y_LABEL: str, diff --git a/dvc/utils/plots.py b/dvc/utils/plots.py index fe88a2622..80d5c145a 100644 --- a/dvc/utils/plots.py +++ b/dvc/utils/plots.py @@ -1,2 +1,6 @@ -def get_plot_id(plot_id: str, config_file_path: str = ""): - return f"{config_file_path}::{plot_id}" if config_file_path else plot_id +def get_plot_id(config_plot_id: str, config_file_path: str = ""): + return ( + f"{config_file_path}::{config_plot_id}" + if config_file_path + else config_plot_id + ) diff --git a/tests/func/plots/test_show.py b/tests/func/plots/test_show.py index e2546f412..5cb55841c 100644 --- a/tests/func/plots/test_show.py +++ b/tests/func/plots/test_show.py @@ -336,16 +336,15 @@ def test_collect_non_existing_dir(tmp_dir, dvc, run_copy_metrics): @pytest.mark.parametrize( "plot_config,expected_datafiles", [ - # TODO - enable providing data files for x - # ( - # { - # "comparison": { - # "x": {"data1.json": "a"}, - # "y": {"sub/dir/data2.json": "b"}, - # } - # }, - # ["data1.json", os.path.join("sub", "dir", "data2.json")], - # ), + ( + { + "comparison": { + "x": {"data1.json": "a"}, + "y": {"sub/dir/data2.json": "b"}, + } + }, + ["data1.json", os.path.join("sub", "dir", "data2.json")], + ), ( {"data1.json": {"x": "c", "y": "a", "title": "File as key test"}}, ["data1.json"], diff --git a/tests/integration/plots/test_plots.py b/tests/integration/plots/test_plots.py index 1969e1487..193c58871 100644 --- a/tests/integration/plots/test_plots.py +++ b/tests/integration/plots/test_plots.py @@ -456,9 +456,7 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): "dvc.yaml::confusion_train_vs_test", ], ) - assert html_result["dvc.yaml::linear_train_vs_test"]["data"][ - "values" - ] == _update_datapoints( + ble = _update_datapoints( plots["data"]["linear_train.json"], { REVISION_FIELD: "linear_train.json", @@ -470,4 +468,7 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots): }, ) + assert ( + html_result["dvc.yaml::linear_train_vs_test"]["data"]["values"] == ble + ) # TODO check json results once vscode is able to handle flexible plots diff --git a/tests/unit/render/test_image_converter.py b/tests/unit/render/test_image_converter.py index 8912fad85..97497d8c8 100644 --- a/tests/unit/render/test_image_converter.py +++ b/tests/unit/render/test_image_converter.py @@ -3,10 +3,9 @@ from dvc.render.converter.image import ImageConverter def test_image_converter_no_out(): - data = b"content" - converter = ImageConverter() - - datapoints, _ = converter.convert(data, "r", "image.png") + data = {"image.png": b"content"} + converter = ImageConverter("image.png", data) + datapoints, _ = converter.flat_datapoints("r") assert datapoints[0] == { REVISION_FIELD: "r", @@ -16,10 +15,10 @@ def test_image_converter_no_out(): def test_image_converter_with_out(tmp_dir): - data = b"content" - converter = ImageConverter({"out": tmp_dir / "foo"}) + data = {"image.png": b"content"} + converter = ImageConverter("image.png", data, {"out": tmp_dir / "foo"}) - datapoints, _ = converter.convert(data, "r", "image.png") + datapoints, _ = converter.flat_datapoints("r") assert datapoints[0] == { REVISION_FIELD: "r", @@ -32,10 +31,10 @@ def test_image_converter_with_out(tmp_dir): def test_image_converter_with_slash_in_revision(tmp_dir): """Regression test for #7934""" - data = b"content" - converter = ImageConverter({"out": tmp_dir / "foo"}) + data = {"image.png": b"content"} + converter = ImageConverter("image.png", data, {"out": tmp_dir / "foo"}) - datapoints, _ = converter.convert(data, "feature/r", "image.png") + datapoints, _ = converter.flat_datapoints("feature/r") assert datapoints[0] == { REVISION_FIELD: "feature/r", diff --git a/tests/unit/render/test_match.py b/tests/unit/render/test_match.py index 85bb3b2e4..0b3362e96 100644 --- a/tests/unit/render/test_match.py +++ b/tests/unit/render/test_match.py @@ -111,6 +111,8 @@ def test_match_renderers(mocker): "title": "config_file_1::plot_id_1", "x": "x", "y": "y", + "x_label": "x", + "y_label": "y", } diff --git a/tests/unit/render/test_vega_converter.py b/tests/unit/render/test_vega_converter.py index 6117cbeae..6a99b5d96 100644 --- a/tests/unit/render/test_vega_converter.py +++ b/tests/unit/render/test_vega_converter.py @@ -3,38 +3,7 @@ from collections import OrderedDict import pytest from dvc.render import VERSION_FIELD -from dvc.render.converter.vega import ( - FieldsNotFoundError, - PlotDataStructureError, - VegaConverter, - _filter_fields, - _find_first_list, - _lists, -) - - -def test_find_first_list_in_dict(): - m1 = [{"accuracy": 1, "loss": 2}, {"accuracy": 3, "loss": 4}] - m2 = [{"x": 1}, {"x": 2}] - dmetric = OrderedDict([("t1", m1), ("t2", m2)]) - - assert _find_first_list(dmetric, fields=set()) == m1 - assert _find_first_list(dmetric, fields={"x"}) == m2 - - with pytest.raises(PlotDataStructureError): - _find_first_list(dmetric, fields={"foo"}) - - -def test_filter_fields(): - m = [{"accuracy": 1, "loss": 2}, {"accuracy": 3, "loss": 4}] - - assert _filter_fields(m, fields=set()) == m - - expected = [{"accuracy": 1}, {"accuracy": 3}] - assert _filter_fields(m, fields={"accuracy"}) == expected - - with pytest.raises(FieldsNotFoundError): - _filter_fields(m, fields={"bad_field"}) +from dvc.render.converter.vega import VegaConverter, _lists @pytest.mark.parametrize( @@ -58,7 +27,7 @@ def test_finding_lists(dictionary, expected_result): "input_data,properties,expected_datapoints,expected_properties", [ pytest.param( - {"metric": [{"v": 1}, {"v": 2}]}, + {"f": {"metric": [{"v": 1}, {"v": 2}]}}, {}, [ { @@ -80,41 +49,11 @@ def test_finding_lists(dictionary, expected_result): }, }, ], - {"x": "step", "y": "v"}, + {"x": "step", "y": "v", "x_label": "step", "y_label": "v"}, id="default_x_y", ), pytest.param( - {"metric": [{"v": 1, "v2": 0.1}, {"v": 2, "v2": 0.2}]}, - {"fields": {"v"}}, - [ - { - "v": 1, - "step": 0, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, - }, - { - "v": 2, - "step": 1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v", - }, - }, - ], - { - "x": "step", - "y": "v", - "fields": {"v", "step"}, - }, - id="filter_fields", - ), - pytest.param( - {"metric": [{"v": 1, "v2": 0.1}, {"v": 2, "v2": 0.2}]}, + {"f": {"metric": [{"v": 1, "v2": 0.1}, {"v": 2, "v2": 0.2}]}}, {"x": "v", "y": "v2"}, [ { @@ -136,52 +75,24 @@ def test_finding_lists(dictionary, expected_result): }, }, ], - {"x": "v", "y": "v2"}, + {"x": "v", "y": "v2", "x_label": "v", "y_label": "v2"}, id="choose_x_y", ), pytest.param( { - "metric": [ - {"v": 1, "v2": 0.1, "v3": 0.01, "v4": 0.001}, - {"v": 2, "v2": 0.2, "v3": 0.02, "v4": 0.002}, - ] - }, - {"x": "v3", "y": "v4", "fields": {"v"}}, - [ - { - "v": 1, - "v3": 0.01, - "v4": 0.001, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v4", + "f": { + "some": "noise", + "very": { + "nested": { + "metric": [ + {"v": 1, "v2": 0.1}, + {"v": 2, "v2": 0.2}, + ] + } }, - }, - { - "v": 2, - "v3": 0.02, - "v4": 0.002, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v4", - }, - }, - ], - {"x": "v3", "y": "v4", "fields": {"v", "v3", "v4"}}, - id="append_x_y_to_fields", - ), - pytest.param( - { - "some": "noise", - "very": { - "nested": { - "metric": [{"v": 1, "v2": 0.1}, {"v": 2, "v2": 0.2}] - } - }, + } }, - {"x": "v", "y": "v2"}, + {"x": "v", "y": "v2", "x_label": "x", "y_label": "y"}, [ { "v": 1, @@ -202,61 +113,128 @@ def test_finding_lists(dictionary, expected_result): }, }, ], - {"x": "v", "y": "v2"}, + {"x": "v", "y": "v2", "x_label": "x", "y_label": "y"}, id="find_in_nested_structure", ), pytest.param( - {"metric": [{"v": 1, "v2": 0.1}, {"v": 2, "v2": 0.2}]}, + {"f": {"metric": [{"v": 1, "v2": 0.1}, {"v": 2, "v2": 0.2}]}}, {"y": {"f": ["v", "v2"]}}, [ { - "v": 1, - "v2": 0.1, VERSION_FIELD: { "revision": "r", "filename": "f", "field": "v", }, "dvc_inferred_y_value": 1, - "step": 0, - }, - { "v": 1, "v2": 0.1, - VERSION_FIELD: { - "revision": "r", - "filename": "f", - "field": "v2", - }, - "dvc_inferred_y_value": 0.1, "step": 0, }, { - "v": 2, - "v2": 0.2, VERSION_FIELD: { "revision": "r", "filename": "f", "field": "v", }, "dvc_inferred_y_value": 2, + "v": 2, + "v2": 0.2, "step": 1, }, { - "v": 2, - "v2": 0.2, VERSION_FIELD: { "revision": "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", + }, + "v": 2, + "v2": 0.2, "dvc_inferred_y_value": 0.2, "step": 1, }, ], - {"x": "step", "y": "dvc_inferred_y_value", "y_label": "y"}, + { + "x": "step", + "y": "dvc_inferred_y_value", + "y_label": "y", + "x_label": "step", + }, id="y_def_list", ), + pytest.param( + { + "f": { + "metric": [{"v": 1}, {"v": 2}], + "other_metric": [{"z": 3}, {"z": 4}], + } + }, + {"y": {"f": ["v", "z"]}}, + [ + { + "dvc_inferred_y_value": 1, + "z": 3, + "v": 1, + "step": 0, + VERSION_FIELD: { + "revision": "r", + "filename": "f", + "field": "v", + }, + }, + { + "dvc_inferred_y_value": 2, + "z": 4, + "step": 1, + "v": 2, + VERSION_FIELD: { + "revision": "r", + "filename": "f", + "field": "v", + }, + }, + { + "dvc_inferred_y_value": 3, + "v": 1, + "z": 3, + "step": 0, + VERSION_FIELD: { + "revision": "r", + "filename": "f", + "field": "z", + }, + }, + { + "dvc_inferred_y_value": 4, + "v": 2, + "z": 4, + "step": 1, + VERSION_FIELD: { + "revision": "r", + "filename": "f", + "field": "z", + }, + }, + ], + { + "x": "step", + "y": "dvc_inferred_y_value", + "y_label": "y", + "x_label": "step", + }, + id="multi_source_json", + ), ], ) def test_convert( @@ -265,32 +243,8 @@ def test_convert( expected_datapoints, expected_properties, ): - converter = VegaConverter(properties) - datapoints, resolved_properties = converter.convert( - data=input_data, revision="r", filename="f" - ) + converter = VegaConverter("f", input_data, properties) + datapoints, resolved_properties = converter.flat_datapoints("r") assert datapoints == expected_datapoints assert resolved_properties == expected_properties - - -def test_convert_skip_step(): - converter = VegaConverter() - converter.skip_step("append_index") - converter.skip_step("generate_y") - - datapoints, resolved_properties = converter.convert( - data={"a": "b", "metric": [{"v": 1}, {"v": 2}]}, - revision="r", - filename="f", - ) - - assert datapoints == [ - { - "v": 1, - }, - { - "v": 2, - }, - ] - assert resolved_properties == {"x": "step", "y": "v"}