Repository: iterative/dvc

Issue:
`plots modify` is not preserving `meta`
I think, I implemented this. But, somehow it's not working. Also need to take a look if comments are preserved.

Discussion:

Patch:
diff --git a/dvc/dvcfile.py b/dvc/dvcfile.py
index df8a61b7c..023d69517 100644
--- a/dvc/dvcfile.py
+++ b/dvc/dvcfile.py
@@ -208,6 +208,8 @@ class PipelineFile(FileMixin):
 
         if existing_entry:
             orig_stage_data = data["stages"][stage.name]
+            if "meta" in orig_stage_data:
+                stage_data[stage.name]["meta"] = orig_stage_data["meta"]
             apply_diff(stage_data[stage.name], orig_stage_data)
         else:
             data["stages"].update(stage_data)
diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py
index fd092bdf1..a4340859e 100644
--- a/dvc/repo/plots/__init__.py
+++ b/dvc/repo/plots/__init__.py
@@ -84,6 +84,11 @@ class Plots:
     def modify(self, path, props=None, unset=None):
         from dvc.dvcfile import Dvcfile
 
+        props = props or {}
+        template = props.get("template")
+        if template:
+            self.repo.plot_templates.get_template(template)
+
         (out,) = self.repo.find_outs_by_path(path)
 
         # This out will become a plot unless it is one already
@@ -92,7 +97,7 @@ class Plots:
 
         for field in unset or ():
             out.plot.pop(field, None)
-        out.plot.update(props or {})
+        out.plot.update(props)
 
         # Empty dict will move it to non-plots
         if not out.plot:
@@ -101,7 +106,7 @@ class Plots:
         out.verify_metric()
 
         dvcfile = Dvcfile(self.repo, out.stage.path)
-        dvcfile.dump(out.stage, update_pipeline=True)
+        dvcfile.dump(out.stage, update_pipeline=True, no_lock=True)
 
 
 def _collect_plots(repo, targets=None, rev=None):


Test patch:
diff --git a/tests/func/plots/__init__.py b/tests/func/plots/__init__.py
index abeba7bda..41851900f 100644
--- a/tests/func/plots/__init__.py
+++ b/tests/func/plots/__init__.py
@@ -5,11 +5,11 @@ import pytest
 
 @pytest.fixture
 def run_copy_metrics(tmp_dir, run_copy):
-    def run(file1, file2, commit=None, tag=None, **kwargs):
+    def run(file1, file2, commit=None, tag=None, single_stage=True, **kwargs):
         stage = tmp_dir.dvc.run(
             cmd=f"python copy.py {file1} {file2}",
             deps=[file1],
-            single_stage=True,
+            single_stage=single_stage,
             **kwargs,
         )
 
diff --git a/tests/func/plots/test_plots.py b/tests/func/plots/test_plots.py
index 46b145deb..33a33d047 100644
--- a/tests/func/plots/test_plots.py
+++ b/tests/func/plots/test_plots.py
@@ -9,6 +9,7 @@ import pytest
 import yaml
 from funcy import first
 
+from dvc.dvcfile import PIPELINE_LOCK
 from dvc.repo.plots.data import (
     JSONPlotData,
     NoMetricInHistoryError,
@@ -17,6 +18,7 @@ from dvc.repo.plots.data import (
     YAMLPlotData,
 )
 from dvc.repo.plots.template import NoFieldInDataError, TemplateNotFoundError
+from dvc.utils import relpath
 
 
 def _write_csv(metric, filename, header=True):
@@ -538,3 +540,48 @@ def test_load_metric_from_dict_yaml(tmp_dir):
         d["rev"] = "revision"
 
     assert list(map(dict, plot_data.to_datapoints())) == expected
+
+
+def test_plots_modify_existing_template(
+    tmp_dir, dvc, run_copy_metrics, custom_template
+):
+    metric = [{"a": 1, "b": 2}, {"a": 2, "b": 3}]
+    _write_json(tmp_dir, metric, "metric_t.json")
+    stage = run_copy_metrics(
+        "metric_t.json",
+        "metric.json",
+        plots_no_cache=["metric.json"],
+        name="copy-metrics",
+        single_stage=False,
+    )
+    dvc.plots.modify(
+        "metric.json", props={"template": relpath(custom_template)}
+    )
+    stage = stage.reload()
+    assert stage.outs[0].plot == {"template": relpath(custom_template)}
+
+
+def test_plots_modify_should_not_change_lockfile(
+    tmp_dir, dvc, run_copy_metrics, custom_template
+):
+    _write_json(tmp_dir, [{"a": 1, "b": 2}], "metric_t.json")
+    run_copy_metrics(
+        "metric_t.json",
+        "metric.json",
+        plots_no_cache=["metric.json"],
+        name="copy-metrics",
+        single_stage=False,
+    )
+
+    (tmp_dir / PIPELINE_LOCK).unlink()
+    dvc.plots.modify(
+        "metric.json", props={"template": relpath(custom_template)}
+    )
+    assert not (tmp_dir / PIPELINE_LOCK).exists()
+
+
+def test_plots_modify_not_existing_template(dvc):
+    with pytest.raises(TemplateNotFoundError):
+        dvc.plots.modify(
+            "metric.json", props={"template": "not-existing-template.json"}
+        )
diff --git a/tests/func/test_dvcfile.py b/tests/func/test_dvcfile.py
index 6ba4ed72a..6206f12e3 100644
--- a/tests/func/test_dvcfile.py
+++ b/tests/func/test_dvcfile.py
@@ -290,3 +290,37 @@ def test_remove_stage_preserves_comment(tmp_dir, dvc, run_copy):
         "# This copies 'foo' text to 'foo' file."
         in (tmp_dir / PIPELINE_FILE).read_text()
     )
+
+
+def test_dvcfile_dump_preserves_meta(tmp_dir, dvc, run_copy):
+    tmp_dir.gen("foo", "foo")
+    stage = run_copy("foo", "bar", name="run_copy")
+    dvcfile = stage.dvcfile
+
+    data = dvcfile._load()[0]
+    metadata = {"name": "copy-file"}
+    data["stages"]["run_copy"]["meta"] = metadata
+    dump_yaml(dvcfile.path, data)
+
+    dvcfile.dump(stage, update_pipeline=True)
+    assert dvcfile._load()[0] == data
+    assert dvcfile._load()[0]["stages"]["run_copy"]["meta"] == metadata
+
+
+def test_dvcfile_dump_preserves_comments(tmp_dir, dvc):
+    text = textwrap.dedent(
+        """\
+        stages:
+          generate-foo:
+            cmd: echo foo > foo
+            # This copies 'foo' text to 'foo' file.
+            outs:
+            - foo"""
+    )
+    tmp_dir.gen("dvc.yaml", text)
+    stage = dvc.get_stage(name="generate-foo")
+    stage.outs[0].use_cache = False
+    dvcfile = stage.dvcfile
+
+    dvcfile.dump(stage, update_pipeline=True)
+    assert dvcfile._load()[1] == (text + ":\n\tcache: false\n".expandtabs())