Skip to content

Commit fe1d7e9

Browse files
authored
params: fix skipping of params dvc.lock when it's a falsy value (#4185)
The falsy values were being skipped when merging params values from dvc.yaml and dvc.lock, showing the values as always changed. These values were never added to `info`, but were found in `self.params` making them appear as changed. Fixes #4184
1 parent 0899b27 commit fe1d7e9

File tree

3 files changed

+28
-9
lines changed

3 files changed

+28
-9
lines changed

dvc/dependency/param.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,8 @@ def fill_values(self, values=None):
4545
if not values:
4646
return
4747
for param in self.params:
48-
value = values.get(param)
49-
if value:
50-
self.info[param] = value
48+
if param in values:
49+
self.info[param] = values[param]
5150

5251
def save(self):
5352
super().save()

tests/func/test_run_multistage.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ def test_run_params_default(tmp_dir, dvc):
244244
params=["nested.nested1.nested2"],
245245
cmd="cat params.yaml",
246246
)
247-
isinstance(stage.deps[0], ParamsDependency)
247+
assert isinstance(stage.deps[0], ParamsDependency)
248248
assert stage.deps[0].params == ["nested.nested1.nested2"]
249249

250250
lockfile = stage.dvcfile._lockfile

tests/unit/dependency/test_params.py

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@
44
from dvc.dependency import ParamsDependency, loadd_from, loads_params
55
from dvc.dependency.param import BadParamFileError, MissingParamsError
66
from dvc.stage import Stage
7+
from dvc.utils.yaml import load_yaml
78

89
PARAMS = {
910
"foo": 1,
1011
"bar": 53.135,
1112
"baz": "str",
1213
"qux": None,
1314
}
15+
DEFAULT_PARAMS_FILE = ParamsDependency.DEFAULT_PARAMS_FILE
1416

1517

1618
def test_loads_params(dvc):
@@ -63,15 +65,15 @@ def test_loadd_from(dvc):
6365
def test_dumpd_with_info(dvc):
6466
dep = ParamsDependency(Stage(dvc), None, PARAMS)
6567
assert dep.dumpd() == {
66-
"path": "params.yaml",
68+
"path": DEFAULT_PARAMS_FILE,
6769
"params": PARAMS,
6870
}
6971

7072

7173
def test_dumpd_without_info(dvc):
7274
dep = ParamsDependency(Stage(dvc), None, list(PARAMS.keys()))
7375
assert dep.dumpd() == {
74-
"path": "params.yaml",
76+
"path": DEFAULT_PARAMS_FILE,
7577
"params": list(PARAMS.keys()),
7678
}
7779

@@ -82,15 +84,16 @@ def test_read_params_nonexistent_file(dvc):
8284

8385

8486
def test_read_params_unsupported_format(tmp_dir, dvc):
85-
tmp_dir.gen("params.yaml", b"\0\1\2\3\4\5\6\7")
87+
tmp_dir.gen(DEFAULT_PARAMS_FILE, b"\0\1\2\3\4\5\6\7")
8688
dep = ParamsDependency(Stage(dvc), None, ["foo"])
8789
with pytest.raises(BadParamFileError):
8890
dep.read_params()
8991

9092

9193
def test_read_params_nested(tmp_dir, dvc):
9294
tmp_dir.gen(
93-
"params.yaml", yaml.dump({"some": {"path": {"foo": ["val1", "val2"]}}})
95+
DEFAULT_PARAMS_FILE,
96+
yaml.dump({"some": {"path": {"foo": ["val1", "val2"]}}}),
9497
)
9598
dep = ParamsDependency(Stage(dvc), None, ["some.path.foo"])
9699
assert dep.read_params() == {"some.path.foo": ["val1", "val2"]}
@@ -103,7 +106,24 @@ def test_save_info_missing_config(dvc):
103106

104107

105108
def test_save_info_missing_param(tmp_dir, dvc):
106-
tmp_dir.gen("params.yaml", "bar: baz")
109+
tmp_dir.gen(DEFAULT_PARAMS_FILE, "bar: baz")
107110
dep = ParamsDependency(Stage(dvc), None, ["foo"])
108111
with pytest.raises(MissingParamsError):
109112
dep.save_info()
113+
114+
115+
@pytest.mark.parametrize(
116+
"param_value",
117+
["", "false", "[]", "{}", "null", "no", "off"]
118+
# we use pyyaml to load params.yaml, which only supports YAML 1.1
119+
# so, some of the above are boolean values
120+
)
121+
def test_params_with_false_values(tmp_dir, dvc, param_value):
122+
key = "param"
123+
dep = ParamsDependency(Stage(dvc), DEFAULT_PARAMS_FILE, [key])
124+
(tmp_dir / DEFAULT_PARAMS_FILE).write_text(f"{key}: {param_value}")
125+
126+
dep.fill_values(load_yaml(DEFAULT_PARAMS_FILE))
127+
128+
with dvc.state:
129+
assert dep.status() == {}

0 commit comments

Comments
 (0)