Skip to content
/ dvc Public
forked from iterative/dvc

Commit

Permalink
parametrization: convert bool to string as "true"/"false"
Browse files Browse the repository at this point in the history
Previously, Python's `str()` was being used that
resulted in boolean transformed into "True"/"False".

Fixes iterative#4996
  • Loading branch information
skshetry committed Dec 1, 2020
1 parent bc1d105 commit 96c67df
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 2 deletions.
13 changes: 12 additions & 1 deletion dvc/parsing/interpolate.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import re
import typing
from functools import singledispatch

from pyparsing import (
CharsNotIn,
Expand Down Expand Up @@ -57,6 +58,16 @@ def format_and_raise_parse_error(exc):
raise ParseError(_format_exc_msg(exc))


@singledispatch
def to_str(obj):
return str(obj)


@to_str.register(bool)
def _(obj: bool):
return "true" if obj else "false"


def _format_exc_msg(exc: ParseException):
exc.loc += 2 # 2 because we append `${` at the start of expr below

Expand Down Expand Up @@ -103,7 +114,7 @@ def str_interpolate(template: str, matches: "List[Match]", context: "Context"):
raise ParseError(
f"Cannot interpolate data of type '{type(value).__name__}'"
)
buf += template[index:start] + str(value)
buf += template[index:start] + to_str(value)
index = end
buf += template[index:]
# regex already backtracks and avoids any `${` starting with
Expand Down
6 changes: 5 additions & 1 deletion tests/func/test_stage_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,16 @@ def test_set(tmp_dir, dvc, value):
}
}
resolver = DataResolver(dvc, PathInfo(str(tmp_dir)), d)
if isinstance(value, bool):
stringified_value = "true" if value else "false"
else:
stringified_value = str(value)
assert_stage_equal(
resolver.resolve(),
{
"stages": {
"build": {
"cmd": f"python script.py --thresh {value}",
"cmd": f"python script.py --thresh {stringified_value}",
"always_changed": value,
}
}
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,17 @@ def test_resolve_resolves_dict_keys():
}


def test_resolve_resolves_boolean_value():
d = {"enabled": True, "disabled": False}
context = Context(d)

assert context.resolve_str("${enabled}") is True
assert context.resolve_str("${disabled}") is False

assert context.resolve_str("--flag ${enabled}") == "--flag true"
assert context.resolve_str("--flag ${disabled}") == "--flag false"


def test_merge_from_raises_if_file_not_exist(tmp_dir, dvc):
context = Context(foo="bar")
with pytest.raises(ParamsFileNotFound):
Expand Down

0 comments on commit 96c67df

Please sign in to comment.