plots: fix plots nested configuration

dvc/repo/plots/__init__.py

@@ -448,6 +448,10 @@ def _resolve_definitions(
     config_path = os.fspath(config_path)
     config_dir = fs.dirname(config_path)
     result: dict[str, dict] = {}
+
+    plot_ids_parents = [
+        _normpath(fs.join(config_dir, plot_id)) for plot_id in definitions
+    ]
     for plot_id, plot_props in definitions.items():
         if plot_props is None:
             plot_props = {}
@@ -457,6 +461,12 @@ def _resolve_definitions(
                 unpacked = unpack_if_dir(
                     fs, data_path, props=plot_props | props, onerror=onerror
                 )
+                # use config for parent directory with most specific definition
+                unpacked["data"] = {
+                    k: v
+                    for k, v in unpacked["data"].items()
+                    if _closest_parent(fs, k, plot_ids_parents) == data_path
+                }
                 dpath.merge(result, unpacked)
         elif _matches(targets, config_path, plot_id):
             adjusted_props = _adjust_sources(fs, plot_props, config_dir)
@@ -465,6 +475,15 @@ def _resolve_definitions(
     return result
 
 
+def _closest_parent(fs, path, parents):
+    best_result = ""
+    for parent in parents:
+        common_path = fs.commonpath([path, parent])
+        if len(common_path) > len(best_result):
+            best_result = common_path
+    return best_result
+
+
 def _collect_pipeline_files(repo, targets: list[str], props, onerror=None):
     result: dict[str, dict] = {}
     top_plots = repo.index._plots
@@ -519,7 +538,7 @@ def unpack_if_dir(fs, path, props: dict[str, str], onerror: Optional[Callable] =
 
     if "data" in unpacked:
         for subpath in unpacked["data"]:
-            result["data"].update({subpath: props})
+            result["data"].update({subpath: props.copy()})
     else:
         result.update(unpacked)
 

tests/func/plots/test_collect.py

@@ -0,0 +1,63 @@
+import dpath
+
+from dvc.repo.plots import Plots
+
+
+def test_subdir_config_not_overwritten_by_parents(tmp_dir, scm, dvc):
+    plot_data = [
+        {"x": 1, "y": 0.1},
+        {"x": 2, "y": 0.2},
+        {"x": 3, "y": 0.3},
+    ]
+    subdir_plot_data = [
+        {"x": 1, "y": 0.2, "z": 0.1},
+        {"x": 2, "y": 0.3, "z": 0.2},
+        {"x": 3, "y": 0.4, "z": 0.3},
+    ]
+
+    (tmp_dir / "plots").mkdir()
+    (tmp_dir / "plots" / "subdir").mkdir()
+
+    (tmp_dir / "plots" / "plot.json").dump_json(plot_data)
+    (tmp_dir / "plots" / "subdir" / "plot.json").dump_json(subdir_plot_data)
+
+    plots_config = [
+        {
+            "plots/subdir/": {
+                "x": "z",
+                "y": "x",
+            }
+        },
+        {"plots": {"x": "x", "y": "y"}},
+        {
+            "subdir axis defined by filename": {
+                "x": {"plots/subdir/plot.json": "x"},
+                "y": {"plots/subdir/plot.json": "y"},
+            }
+        },
+    ]
+
+    from dvc.utils.serialize import modify_yaml
+
+    with modify_yaml("dvc.yaml") as dvcfile_content:
+        dvcfile_content["plots"] = plots_config
+
+    scm.add(
+        [
+            "plots/plot.json",
+            "plots/subdir/plot.json",
+            "dvc.yaml",
+        ]
+    )
+    scm.commit("add data sources")
+
+    plots = next(Plots(dvc).collect())
+
+    assert dpath.get(plots, "workspace/definitions/data/dvc.yaml/data") == {
+        "plots/plot.json": {"x": "x", "y": "y"},
+        "plots/subdir/plot.json": {"x": "z", "y": "x"},
+        "subdir axis defined by filename": {
+            "x": {"plots/subdir/plot.json": "x"},
+            "y": {"plots/subdir/plot.json": "y"},
+        },
+    }

tests/integration/plots/conftest.py

@@ -86,15 +86,27 @@ def make():
 @pytest.fixture
 def repo_with_config_plots(tmp_dir, scm, dvc, run_copy_metrics):
     def make():
+        # test subdir functionality
+        linear_subdir_a_v1 = [
+            {"x": 1, "y": 0.2},
+            {"x": 2, "y": 0.3},
+            {"x": 3, "y": 0.4},
+        ]
+        # test subdir default values for x = step
+        linear_subdir_b_v1 = [
+            {"step": 1, "y": 0.2},
+            {"step": 2, "y": 0.3},
+            {"step": 3, "y": 0.4},
+        ]
         linear_train_v1 = [
             {"x": 1, "y": 0.1},
             {"x": 2, "y": 0.2},
             {"x": 3, "y": 0.3},
         ]
         linear_test_v1 = [
-            {"x": 1, "y": 0.2},
-            {"x": 2, "y": 0.3},
-            {"x": 3, "y": 0.4},
+            {"x": 1, "y": 0.3},
+            {"x": 2, "y": 0.4},
+            {"x": 3, "y": 0.5},
         ]
 
         confusion_train_v1 = [
@@ -110,13 +122,17 @@ def make():
             {"actual": 1, "predicted": 0},
         ]
 
+        (tmp_dir / "linear_subdir_a_src.json").dump_json(linear_subdir_a_v1)
+        (tmp_dir / "linear_subdir_b_src.json").dump_json(linear_subdir_b_v1)
         (tmp_dir / "linear_train_src.json").dump_json(linear_train_v1)
         (tmp_dir / "linear_test_src.json").dump_json(linear_test_v1)
         (tmp_dir / "confusion_train_src.json").dump_json(confusion_train_v1)
         (tmp_dir / "confusion_test_src.json").dump_json(confusion_test_v1)
 
         scm.add(
             [
+                "linear_subdir_a_src.json",
+                "linear_subdir_b_src.json",
                 "linear_train_src.json",
                 "linear_test_src.json",
                 "confusion_train_src.json",
@@ -125,6 +141,23 @@ def make():
         )
         scm.commit("add data sources")
 
+        (tmp_dir / "subdirA").mkdir()
+        (tmp_dir / "subdirB").mkdir()
+
+        run_copy_metrics(
+            "linear_subdir_a_src.json",
+            "subdirA/linear_subdir.json",
+            name="linear_subdir",
+            outs=["subdirA/linear_subdir.json"],
+            commit="linear_subdir",
+        )
+        run_copy_metrics(
+            "linear_subdir_b_src.json",
+            "subdirB/linear_subdir.json",
+            name="linear_subdir",
+            outs=["subdirB/linear_subdir.json"],
+            commit="linear_subdir",
+        )
         run_copy_metrics(
             "linear_train_src.json",
             "linear_train.json",
@@ -153,7 +186,16 @@ def make():
             outs=["confusion_test.json"],
             commit="confusion_test",
         )
-        plots_config = [
+
+        subdir_a_config = {
+            "x": "x",
+            "y": "y",
+            "title": "subdir plots with x and y defined",
+        }
+
+        subdir_b_config = {"title": "subdir plots with default x and y"}
+
+        other_plots_config = [
             {
                 "linear_train_vs_test": {
                     "x": "x",
@@ -176,18 +218,35 @@ def make():
         from dvc.utils.serialize import modify_yaml
 
         with modify_yaml("dvc.yaml") as dvcfile_content:
-            dvcfile_content["plots"] = plots_config
+            dvcfile_content["plots"] = [
+                {"subdirA": subdir_a_config},
+                {"subdirB": subdir_b_config},
+                *other_plots_config,
+            ]
 
         scm.add(["dvc.yaml", "dvc.lock"])
         scm.commit("commit dvc files")
+
+        # remove generate .gitignore that are considered as plot otherwise
+        (tmp_dir / "subdirA" / ".gitignore").unlink()
+        (tmp_dir / "subdirB" / ".gitignore").unlink()
+
         yield {
             "data": {
+                "subdirA/linear_subdir.json": linear_subdir_a_v1,
+                "subdirB/linear_subdir.json": linear_subdir_b_v1,
                 "linear_train.json": linear_train_v1,
                 "linear_test.json": linear_test_v1,
                 "confusion_train.json": confusion_train_v1,
                 "confusion_test.json": confusion_test_v1,
             },
-            "configs": {"dvc.yaml": plots_config},
+            "configs": {
+                "dvc.yaml": [
+                    {"subdirA/linear_subdir.json": subdir_a_config},
+                    {"subdirB/linear_subdir.json": subdir_b_config},
+                    *other_plots_config,
+                ]
+            },
         }
 
     return make

tests/integration/plots/test_plots.py

@@ -440,6 +440,8 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots):
     html_result = extract_vega_specs(
         html_path,
         [
+            "subdirA/linear_subdir.json",
+            "subdirB/linear_subdir.json",
             "linear_train_vs_test",
             "confusion_train_vs_test",
         ],
@@ -466,6 +468,22 @@ def test_repo_with_config_plots(tmp_dir, capsys, repo_with_config_plots):
         ]
         == ble
     )
+    assert (
+        html_result["subdirA/linear_subdir.json"]["layer"][1]["encoding"]["x"]["field"]
+        == "x"
+    )
+    assert (
+        html_result["subdirA/linear_subdir.json"]["layer"][1]["encoding"]["y"]["field"]
+        == "y"
+    )
+    assert (
+        html_result["subdirB/linear_subdir.json"]["layer"][1]["encoding"]["x"]["field"]
+        == "step"
+    )
+    assert (
+        html_result["subdirB/linear_subdir.json"]["layer"][1]["encoding"]["y"]["field"]
+        == "y"
+    )
 
 
 @pytest.mark.vscode