From fec1499c67cf27ca45b4943f765431e5a25e0904 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Tue, 3 Nov 2020 17:42:28 +0545 Subject: [PATCH] Move interpolation logic to the Context (#4833) --- dvc/parsing/__init__.py | 8 ++++---- dvc/parsing/context.py | 35 +++++++++++++++++++++++++++++++--- dvc/parsing/interpolate.py | 16 ---------------- tests/unit/test_interpolate.py | 35 +++++++++++++++++----------------- 4 files changed, 53 insertions(+), 41 deletions(-) diff --git a/dvc/parsing/__init__.py b/dvc/parsing/__init__.py index 1ce85c24f..5fffb93be 100644 --- a/dvc/parsing/__init__.py +++ b/dvc/parsing/__init__.py @@ -12,7 +12,6 @@ from dvc.dependency.param import ParamsDependency from dvc.path_info import PathInfo from .context import Context -from .interpolate import resolve if TYPE_CHECKING: from dvc.repo import Repo @@ -105,7 +104,7 @@ class DataResolver: ) with context.track(): - stage_d = resolve(definition, context) + stage_d = context.resolve(definition) params = stage_d.get(PARAMS_KWD, []) + self._resolve_params( context, wdir @@ -125,7 +124,8 @@ class DataResolver: def _resolve_wdir(self, context: Context, wdir: str = None) -> PathInfo: if not wdir: return self.wdir - wdir = resolve(wdir, context) + + wdir = context.resolve(wdir) return self.wdir / str(wdir) def _foreach(self, context: Context, name: str, foreach_data, in_data): @@ -137,7 +137,7 @@ class DataResolver: suffix = str(key if key is not DEFAULT_SENTINEL else value) return self._resolve_stage(c, f"{name}-{suffix}", in_data) - iterable = resolve(foreach_data, context) + iterable = context.resolve(foreach_data) assert isinstance(iterable, (Sequence, Mapping)) and not isinstance( iterable, str diff --git a/dvc/parsing/context.py b/dvc/parsing/context.py index 994e2138a..1c94f7155 100644 --- a/dvc/parsing/context.py +++ b/dvc/parsing/context.py @@ -9,6 +9,7 @@ from typing import Any, List, Optional, Union from funcy import identity from dvc.parsing.interpolate import ( + UNWRAP_DEFAULT, get_matches, is_exact_string, is_interpolated_string, @@ -76,8 +77,8 @@ class Container: data: Union[list, dict] _key_transform = staticmethod(identity) - def __init__(self, meta) -> None: - self.meta = meta or Meta(source=None) + def __init__(self, meta=None) -> None: + self.meta = meta or _default_meta() def _convert(self, key, value): meta = Meta.update_path(self.meta, key) @@ -233,12 +234,40 @@ class Context(CtxDict): raise ValueError(f"Cannot set '{key}', key already exists") if isinstance(value, str): self._check_joined_with_interpolation(key, value) - value = resolve_str(value, self, unwrap=False) + value = self.resolve_str(value, unwrap=False) elif isinstance(value, (Sequence, Mapping)): self._check_not_nested_collection(key, value) self._check_interpolation_collection(key, value) self[key] = value + def resolve(self, src): + """Recursively resolves interpolation and returns resolved data. + + >>> c = Context({"three": 3}) + >>> c.resolve({"lst": [1, 2, "${three}"]}) + {'lst': [1, 2, 3]} + """ + Seq = (list, tuple, set) + if isinstance(src, Mapping): + return {key: self.resolve(value) for key, value in src.items()} + elif isinstance(src, Seq): + return type(src)(map(self.resolve, src)) + elif isinstance(src, str): + return self.resolve_str(src) + return src + + def resolve_str(self, src, unwrap=UNWRAP_DEFAULT): + """Resolves interpolated string to it's original value, + or in case of multiple interpolations, a combined string. + + >>> c = Context({"enabled": True}) + >>> c.resolve_str("${enabled}") + True + >>> c.resolve_str("enabled? ${enabled}") + 'enabled? True' + """ + return resolve_str(src, self, unwrap=unwrap) + @staticmethod def _check_not_nested_collection(key: str, value: SeqOrMap): values = value.values() if isinstance(value, Mapping) else value diff --git a/dvc/parsing/interpolate.py b/dvc/parsing/interpolate.py index 3be15ed49..f122bf66a 100644 --- a/dvc/parsing/interpolate.py +++ b/dvc/parsing/interpolate.py @@ -1,8 +1,5 @@ import re import typing -from collections.abc import Mapping - -from funcy import rpartial if typing.TYPE_CHECKING: from .context import Context @@ -68,16 +65,3 @@ def resolve_str(src: str, context, unwrap=UNWRAP_DEFAULT): # regex already backtracks and avoids any `${` starting with # backslashes(`\`). We just need to replace those by `${`. return src.replace(r"\${", "${") - - -def resolve(src, context): - Seq = (list, tuple, set) - - apply_value = rpartial(resolve, context) - if isinstance(src, Mapping): - return {key: apply_value(value) for key, value in src.items()} - elif isinstance(src, Seq): - return type(src)(map(apply_value, src)) - elif isinstance(src, str): - return resolve_str(src, context) - return src diff --git a/tests/unit/test_interpolate.py b/tests/unit/test_interpolate.py index e43548db3..007dba495 100644 --- a/tests/unit/test_interpolate.py +++ b/tests/unit/test_interpolate.py @@ -3,7 +3,6 @@ from math import inf, pi import pytest from dvc.parsing import Context -from dvc.parsing.interpolate import resolve @pytest.mark.parametrize( @@ -19,7 +18,8 @@ from dvc.parsing.interpolate import resolve "data", [True, 12, pi, None, False, 0, "0", "123", "Foobar", "", inf, 3e4] ) def test_resolve_primitive_values(data, template, var): - assert resolve(template, Context({var: data})) == data + context = Context({var: data}) + assert context.resolve(template) == data @pytest.mark.parametrize( @@ -40,16 +40,15 @@ def test_resolve_primitive_values(data, template, var): ], ) def test_escape(template, expected, mocker): - assert resolve(template, Context({"value": "value"})) == expected + context = Context({"value": "value"}) + assert context.resolve(template) == expected def test_resolve_str(): template = "My name is ${last}, ${first} ${last}" expected = "My name is Bond, James Bond" - assert ( - resolve(template, Context({"first": "James", "last": "Bond"})) - == expected - ) + context = Context({"first": "James", "last": "Bond"}) + assert context.resolve(template) == expected def test_resolve_primitives_dict_access(): @@ -62,12 +61,12 @@ def test_resolve_primitives_dict_access(): } context = Context(data) - assert resolve("${dict.num}", context) == 5 - assert resolve("${dict.string}", context) == "foo" - assert resolve("${dict.nested.float}", context) == pi - assert resolve("${dict.nested.string}", context) == "bar" + assert context.resolve("${dict.num}") == 5 + assert context.resolve("${dict.string}") == "foo" + assert context.resolve("${dict.nested.float}") == pi + assert context.resolve("${dict.nested.string}") == "bar" - assert resolve("Number ${dict.num}", context) == "Number 5" + assert context.resolve("Number ${dict.num}") == "Number 5" def test_resolve_primitives_list_access(): @@ -82,12 +81,12 @@ def test_resolve_primitives_list_access(): } ) - assert resolve("${dict.0.f}", context) == "f" - assert resolve("${dict.1.fo}", context) == "fo" - assert resolve("${dict.2.foo}", context) == "foo" - assert resolve("${dict.3.foo.0}", context) == "f" + assert context.resolve("${dict.0.f}") == "f" + assert context.resolve("${dict.1.fo}") == "fo" + assert context.resolve("${dict.2.foo}") == "foo" + assert context.resolve("${dict.3.foo.0}") == "f" - assert resolve("${ dict.1.fo}${dict.3.foo.1}bar", context) == "foobar" + assert context.resolve("${ dict.1.fo}${dict.3.foo.1}bar") == "foobar" def test_resolve_collection(): @@ -98,4 +97,4 @@ def test_resolve_collection(): ) context = Context(CONTEXT_DATA) - assert resolve(TEMPLATED_DVC_YAML_DATA, context) == RESOLVED_DVC_YAML_DATA + assert context.resolve(TEMPLATED_DVC_YAML_DATA) == RESOLVED_DVC_YAML_DATA