params: allow tracking file

dvc/dependency/__init__.py

@@ -47,16 +47,29 @@ def loads_from(stage, s_list, erepo=None):
 def _merge_params(s_list):
     d = defaultdict(list)
     default_file = ParamsDependency.DEFAULT_PARAMS_FILE
+
+    # figure out completely tracked params file, and ignore specific keys
+    wholly_tracked = set()
+    for key in s_list:
+        if not isinstance(key, dict):
+            continue
+        wholly_tracked.update(k for k, params in key.items() if not params)
+
     for key in s_list:
         if isinstance(key, str):
-            d[default_file].append(key)
+            if default_file not in wholly_tracked:
+                d[default_file].append(key)
             continue
+
         if not isinstance(key, dict):
             msg = "Only list of str/dict is supported. Got: "
             msg += f"'{type(key).__name__}'."
             raise ValueError(msg)
 
         for k, params in key.items():
+            if k in wholly_tracked:
+                d[k] = []
+                continue
             if not isinstance(params, list):
                 msg = "Expected list of params for custom params file "
                 msg += f"'{k}', got '{type(params).__name__}'."

dvc/dependency/param.py

@@ -1,7 +1,9 @@
 import errno
 import logging
 import os
+import typing
 from collections import defaultdict
+from typing import Dict
 
 import dpath.util
 from voluptuous import Any
@@ -37,51 +39,82 @@ class ParamsDependency(Dependency):
     DEFAULT_PARAMS_FILE = "params.yaml"
 
     def __init__(self, stage, path, params=None, repo=None):
-        info = {}
-        self.params = params or []
-        if params:
-            if isinstance(params, list):
-                self.params = params
-            else:
-                assert isinstance(params, dict)
-                self.params = list(params.keys())
-                info = {self.PARAM_PARAMS: params}
-
-        super().__init__(
-            stage,
-            path
-            or os.path.join(stage.repo.root_dir, self.DEFAULT_PARAMS_FILE),
-            info=info,
-            repo=repo,
+        self.params = list(params) if params else []
+        info = (
+            {self.PARAM_PARAMS: params} if isinstance(params, dict) else None
         )
+        repo = repo or stage.repo
+        path = path or os.path.join(repo.root_dir, self.DEFAULT_PARAMS_FILE)
+        super().__init__(stage, path, info=info, repo=repo)
 
     def dumpd(self):
         ret = super().dumpd()
         if not self.hash_info:
-            ret[self.PARAM_PARAMS] = self.params
+            ret[self.PARAM_PARAMS] = self.params or {}
         return ret
 
     def fill_values(self, values=None):
         """Load params values dynamically."""
-        if not values:
+        if values is None:
             return
+
         info = {}
+        if not self.params:
+            info.update(values)
         for param in self.params:
             if param in values:
                 info[param] = values[param]
         self.hash_info = HashInfo(self.PARAM_PARAMS, info)
 
-    def workspace_status(self):
-        status = super().workspace_status()
+    def read_params(
+        self, flatten: bool = True, **kwargs: typing.Any
+    ) -> Dict[str, typing.Any]:
+        try:
+            config = self.read_file()
+        except MissingParamsFile:
+            config = {}
 
-        if status.get(str(self)) == "deleted":
-            return status
+        if not self.params:
+            return config
+
+        ret = {}
+        if flatten:
+            for param in self.params:
+                try:
+                    ret[param] = dpath.util.get(config, param, separator=".")
+                except KeyError:
+                    continue
+            return ret
+
+        from dpath.util import merge
+
+        for param in self.params:
+            merge(
+                ret,
+                dpath.util.search(config, param, separator="."),
+                separator=".",
+            )
+        return ret
+
+    def workspace_status(self):
+        if not self.exists:
+            return {str(self): "deleted"}
+        if self.hash_info.value is None:
+            return {str(self): "new"}
+
+        from funcy import ldistinct
 
         status = defaultdict(dict)
         info = self.hash_info.value if self.hash_info else {}
         actual = self.read_params()
-        for param in self.params:
-            if param not in actual.keys():
+
+        # NOTE: we want to preserve the order of params as specified in the
+        # status. In case of tracking the whole file, the order is top-level
+        # keys in the file and then the keys in the `info` from `dvc.lock`
+        # (which are alphabetically sorted).
+        params = self.params or ldistinct([*actual.keys(), *info.keys()])
+        for param in params:
+            if param not in actual:
                 st = "deleted"
             elif param not in info:
                 st = "new"
@@ -130,35 +163,6 @@ class ParamsDependency(Dependency):
                 f"Unable to read parameters from '{self}'"
             ) from exc
 
-    def _read(self):
-        try:
-            return self.read_file()
-        except MissingParamsFile:
-            return {}
-
-    def read_params_d(self, **kwargs):
-        config = self._read()
-
-        ret = {}
-        for param in self.params:
-            dpath.util.merge(
-                ret,
-                dpath.util.search(config, param, separator="."),
-                separator=".",
-            )
-        return ret
-
-    def read_params(self):
-        config = self._read()
-
-        ret = {}
-        for param in self.params:
-            try:
-                ret[param] = dpath.util.get(config, param, separator=".")
-            except KeyError:
-                pass
-        return ret
-
     def get_hash(self):
         info = self.read_params()
 
@@ -179,12 +183,5 @@ class ParamsDependency(Dependency):
         if not self.isfile and not self.isdir:
             raise self.IsNotFileOrDirError(self)
 
-        if self.is_empty:
-            logger.warning(f"'{self}' is empty.")
-
         self.ignore()
-
-        if self.metric or self.plot:
-            self.verify_metric()
-
         self.hash_info = self.get_hash()

dvc/repo/params/show.py

@@ -66,7 +66,9 @@ def _read_params(
 
     if deps:
         for param in params:
-            params_dict = error_handler(param.read_params_d)(onerror=onerror)
+            params_dict = error_handler(param.read_params)(
+                onerror=onerror, flatten=False
+            )
             if params_dict:
                 res[
                     repo.fs.path.relpath(param.fs_path, os.getcwd())

dvc/stage/serialize.py

@@ -90,18 +90,12 @@ def _serialize_params_keys(params):
     """
     keys = []
     for param_dep in sort_by_path(params):
-        dump = param_dep.dumpd()
-        path, params = dump[PARAM_PATH], dump[PARAM_PARAMS]
-        assert isinstance(params, (dict, list))
         # when on no_exec, params are not filled and are saved as list
-        k = sorted(params.keys() if isinstance(params, dict) else params)
-        if not k:
-            continue
-
-        if path == DEFAULT_PARAMS_FILE:
+        k = sorted(param_dep.params)
+        if k and param_dep.def_path == DEFAULT_PARAMS_FILE:
             keys = k + keys
         else:
-            keys.append({path: k})
+            keys.append({param_dep.def_path: k or None})
     return keys
 
 

tests/func/params/test_diff.py

@@ -1,3 +1,5 @@
+import pytest
+
 from dvc.utils import relpath
 
 
@@ -218,3 +220,24 @@ def test_diff_targeted(tmp_dir, scm, dvc, run_copy):
     assert dvc.params.diff(a_rev="HEAD~2", targets=["other_params.yaml"]) == {
         "other_params.yaml": {"xyz": {"old": "val", "new": "val3"}}
     }
+
+
+@pytest.mark.parametrize("file", ["params.yaml", "other_params.yaml"])
+def test_diff_without_targets_specified(tmp_dir, dvc, scm, file):
+    params_file = tmp_dir / file
+    params_file.dump({"foo": {"bar": "bar"}, "x": "0"})
+    dvc.stage.add(
+        name="test",
+        cmd=f"echo {file}",
+        params=[{file: None}],
+    )
+    scm.add_commit([params_file, "dvc.yaml"], message="foo")
+
+    params_file.dump({"foo": {"bar": "baz"}, "y": "100"})
+    assert dvc.params.diff() == {
+        file: {
+            "foo.bar": {"new": "baz", "old": "bar"},
+            "x": {"new": None, "old": "0"},
+            "y": {"new": "100", "old": None},
+        }
+    }

tests/func/params/test_show.py

@@ -178,3 +178,17 @@ def test_log_errors(tmp_dir, scm, dvc, capsys, file, error_path):
         "DVC failed to load some parameters for following revisions: 'v1'."
         in error
     )
+
+
+@pytest.mark.parametrize("file", ["params.yaml", "other_params.yaml"])
+def test_show_without_targets_specified(tmp_dir, dvc, scm, file):
+    params_file = tmp_dir / file
+    data = {"foo": {"bar": "bar"}, "x": "0"}
+    params_file.dump(data)
+    dvc.stage.add(
+        name="test",
+        cmd=f"echo {file}",
+        params=[{file: None}],
+    )
+
+    assert dvc.params.show() == {"": {"data": {file: {"data": data}}}}

tests/func/test_status.py

@@ -137,3 +137,49 @@ def test_status_outputs(tmp_dir, dvc):
     assert dvc.status(targets=["alice"]) == {
         "alice_bob": [{"changed outs": {"alice": "modified"}}]
     }
+
+
+def test_params_without_targets(tmp_dir, dvc):
+    dvc.stage.add(
+        name="test", cmd="echo params.yaml", params=[{"params.yaml": None}]
+    )
+    assert dvc.status() == {
+        "test": [{"changed deps": {"params.yaml": "deleted"}}]
+    }
+
+    (tmp_dir / "params.yaml").touch()
+    assert dvc.status() == {"test": [{"changed deps": {"params.yaml": "new"}}]}
+
+    dvc.commit("test", force=True)
+    # make sure that we are able to keep track of "empty" contents
+    # and be able to distinguish between no-lock-entry and empty-lock-entry.
+    assert (tmp_dir / "dvc.lock").parse() == {
+        "schema": "2.0",
+        "stages": {
+            "test": {"cmd": "echo params.yaml", "params": {"params.yaml": {}}}
+        },
+    }
+    assert dvc.status() == {}
+
+    (tmp_dir / "params.yaml").dump({"foo": "foo", "bar": "bar"})
+    assert dvc.status() == {
+        "test": [
+            {"changed deps": {"params.yaml": {"bar": "new", "foo": "new"}}}
+        ]
+    }
+    dvc.commit("test", force=True)
+
+    (tmp_dir / "params.yaml").dump({"foo": "foobar", "lorem": "ipsum"})
+    assert dvc.status() == {
+        "test": [
+            {
+                "changed deps": {
+                    "params.yaml": {
+                        "bar": "deleted",
+                        "foo": "modified",
+                        "lorem": "new",
+                    }
+                }
+            }
+        ]
+    }

tests/unit/dependency/test_params.py

@@ -40,6 +40,37 @@ def test_loads_params(dvc):
     assert not deps[2].hash_info
 
 
+def test_loads_params_without_any_specific_targets(dvc):
+    stage = Stage(dvc)
+    deps = loads_params(
+        stage,
+        [
+            "foo",
+            {"params.yaml": None},
+            {"a_file": []},
+            {"b_file": ["baz"]},
+            {"b_file": ["bat"]},
+            {"a_file": ["foobar"]},
+        ],
+    )
+    assert len(deps) == 3
+
+    assert isinstance(deps[0], ParamsDependency)
+    assert deps[0].def_path == ParamsDependency.DEFAULT_PARAMS_FILE
+    assert deps[0].params == []
+    assert not deps[0].hash_info
+
+    assert isinstance(deps[1], ParamsDependency)
+    assert deps[1].def_path == "a_file"
+    assert deps[1].params == []
+    assert not deps[1].hash_info
+
+    assert isinstance(deps[2], ParamsDependency)
+    assert deps[2].def_path == "b_file"
+    assert deps[2].params == ["baz", "bat"]
+    assert not deps[2].hash_info
+
+
 @pytest.mark.parametrize("params", [[3], [{"b_file": "cat"}]])
 def test_params_error(dvc, params):
     with pytest.raises(ValueError):
@@ -199,3 +230,25 @@ def test_params_with_false_values(tmp_dir, dvc, param_value):
     dep.fill_values(load_yaml(DEFAULT_PARAMS_FILE))
 
     assert dep.status() == {}
+
+
+def test_params_status_without_targets(tmp_dir, dvc):
+    params_file = tmp_dir / "params.yaml"
+    dep = ParamsDependency(Stage(dvc), str(params_file), [])
+
+    assert dep.hash_info.value is None
+    assert dep.status() == {"params.yaml": "deleted"}
+
+    params_file.dump({"foo": "foo", "bar": "bar"})
+
+    assert dep.status() == {"params.yaml": "new"}
+
+    dep.fill_values({})
+    assert dep.hash_info.value == {}
+    assert dep.status() == {"params.yaml": {"bar": "new", "foo": "new"}}
+
+    dep.fill_values({"foo": "foobar", "lorem": "ipsum"})
+    assert dep.hash_info.value == {"foo": "foobar", "lorem": "ipsum"}
+    assert dep.status() == {
+        "params.yaml": {"bar": "new", "foo": "modified", "lorem": "deleted"}
+    }

tests/unit/stage/test_serialize_pipeline_file.py

@@ -100,6 +100,25 @@ def test_params_file_sorted(dvc):
     ]
 
 
+def test_params_file_without_targets(dvc):
+    params = [
+        "foo",
+        "bar",
+        {"params.yaml": None},
+        {"custom.yaml": ["wxyz", "pqrs", "baz"]},
+        {"a-file-of-params.yaml": None},
+        {"a-file-of-params.yaml": ["barr"]},
+    ]
+    stage = create_stage(
+        PipelineStage, dvc, outs=["bar"], deps=["foo"], params=params, **kwargs
+    )
+    assert to_pipeline_file(stage)["something"][stage.PARAM_PARAMS] == [
+        {"a-file-of-params.yaml": None},
+        {"custom.yaml": ["baz", "pqrs", "wxyz"]},
+        {"params.yaml": None},
+    ]
+
+
 @pytest.mark.parametrize(
     "typ, extra",
     [("plots", {"plot": True}), ("metrics", {"metric": True}), ("outs", {})],

tests/unit/stage/test_serialize_pipeline_lock.py

@@ -120,6 +120,25 @@ def test_lock_params_no_values_filled(dvc):
     assert to_single_stage_lockfile(stage) == {"cmd": "command"}
 
 
+@pytest.mark.parametrize(
+    "info, expected",
+    [
+        (None, {}),
+        ({}, {}),
+        ({"foo": "foo", "bar": "bar"}, {"bar": "bar", "foo": "foo"}),
+    ],
+)
+def test_lock_params_without_targets(dvc, info, expected):
+    stage = create_stage(
+        PipelineStage, dvc, params=[{"params.yaml": None}], **kwargs
+    )
+    stage.deps[0].fill_values(info)
+    assert to_single_stage_lockfile(stage) == {
+        "cmd": "command",
+        "params": {"params.yaml": OrderedDict(expected)},
+    }
+
+
 @pytest.mark.parametrize("typ", ["plots", "metrics", "outs"])
 def test_lock_outs(dvc, typ):
     stage = create_stage(PipelineStage, dvc, **{typ: ["input"]}, **kwargs)