diff --git a/docs/notebook/Tutorial.ipynb b/docs/notebook/Tutorial.ipynb index 91bdcfc42..1453a8c63 100644 --- a/docs/notebook/Tutorial.ipynb +++ b/docs/notebook/Tutorial.ipynb @@ -521,7 +521,7 @@ "\n", "OmegaConf support variable interpolation, Interpolations are evaluated lazily on access.\n", "\n", - "## Config node interpolation" + "### Config node interpolation" ] }, { @@ -618,46 +618,7 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "Interpolations may be nested, enabling more advanced behavior like dynamically selecting a sub-config:" - ] - }, - { - "cell_type": "code", - "execution_count": 20, - "metadata": { - "pycharm": { - "name": "#%%\n" - }, - "scrolled": true - }, - "outputs": [ - { - "name": "stdout", - "output_type": "stream", - "text": [ - "Default: cfg.plan = plan A\n", - "After selecting plan B: cfg.plan = plan B\n" - ] - } - ], - "source": [ - "cfg = OmegaConf.create(\"\"\"\n", - "plans:\n", - " A: plan A\n", - " B: plan B\n", - "selected_plan: A\n", - "plan: ${plans.${selected_plan}}\n", - "\"\"\")\n", - "print(f\"Default: cfg.plan = {cfg.plan}\")\n", - "cfg.selected_plan = \"B\"\n", - "print(f\"After selecting plan B: cfg.plan = {cfg.plan}\")" - ] - }, - { - "cell_type": "markdown", - "metadata": {}, - "source": [ - "## Environment variable interpolation\n", + "### Environment variable interpolation\n", "\n", "Environment variable interpolation is also supported.\n", "\n", @@ -666,7 +627,7 @@ }, { "cell_type": "code", - "execution_count": 21, + "execution_count": 20, "metadata": {}, "outputs": [], "source": [ @@ -684,7 +645,7 @@ }, { "cell_type": "code", - "execution_count": 22, + "execution_count": 21, "metadata": { "pycharm": { "name": "#%%\n" @@ -709,7 +670,7 @@ }, { "cell_type": "code", - "execution_count": 23, + "execution_count": 22, "metadata": { "pycharm": { "name": "#%%\n" @@ -741,7 +702,7 @@ }, { "cell_type": "code", - "execution_count": 24, + "execution_count": 23, "metadata": {}, "outputs": [ { @@ -768,12 +729,12 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "You can add additional interpolation types using custom resolvers, which take strings as inputs. This example creates a resolver that adds 10 to the given value (note the need to cast it to `int`)." + "You can add additional interpolation types using custom resolvers. This example creates a resolver that adds 10 the the given value." ] }, { "cell_type": "code", - "execution_count": 25, + "execution_count": 24, "metadata": { "pycharm": { "name": "#%%\n" @@ -794,32 +755,6 @@ "print(conf.key)" ] }, - { - "cell_type": "markdown", - "metadata": {}, - "source": [ - "You can take advantage of nested interpolations to perform operations over variables:" - ] - }, - { - "cell_type": "code", - "execution_count": 26, - "metadata": {}, - "outputs": [ - { - "name": "stdout", - "output_type": "stream", - "text": [ - "3\n" - ] - } - ], - "source": [ - "OmegaConf.register_resolver(\"plus_int\", lambda x, y: int(x) + int(y))\n", - "conf = OmegaConf.create({\"a\": 1, \"b\": 2, \"a_plus_b\": \"${plus_int:${a},${b}}\"})\n", - "print(conf.a_plus_b)" - ] - }, { "cell_type": "markdown", "metadata": {}, @@ -843,7 +778,7 @@ }, { "cell_type": "code", - "execution_count": 27, + "execution_count": 25, "metadata": { "pycharm": { "name": "#%%\n" @@ -870,7 +805,7 @@ }, { "cell_type": "code", - "execution_count": 28, + "execution_count": 26, "metadata": { "pycharm": { "name": "#%%\n" @@ -894,7 +829,7 @@ }, { "cell_type": "code", - "execution_count": 29, + "execution_count": 27, "metadata": { "pycharm": { "name": "#%%\n" @@ -954,7 +889,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.6.10" + "version": "3.8.1" }, "pycharm": { "stem_cell": { diff --git a/docs/source/usage.rst b/docs/source/usage.rst index 1e27d42ad..522c367a2 100644 --- a/docs/source/usage.rst +++ b/docs/source/usage.rst @@ -286,23 +286,6 @@ Example: >>> print(type(conf.client.url).__name__) str -Interpolations may be nested, enabling more advanced behavior like dynamically selecting a sub-config: - -.. doctest:: - - >>> cfg = OmegaConf.create(""" - ... plans: - ... A: plan A - ... B: plan B - ... selected_plan: A - ... plan: ${plans.${selected_plan}} - ... """) - >>> print(cfg.plan) # default plan - plan A - >>> cfg.selected_plan = "B" - >>> print(cfg.plan) # new plan - plan B - Environment variable interpolation ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -339,8 +322,8 @@ The following example sets `12345` as the the default value for the `DB_PASSWORD Custom interpolations ^^^^^^^^^^^^^^^^^^^^^ -You can add additional interpolation types using custom resolvers, which take strings as inputs. -This example creates a resolver that adds 10 to the given value (note the need to cast it to `int`). +You can add additional interpolation types using custom resolvers. +This example creates a resolver that adds 10 the the given value. .. doctest:: @@ -353,7 +336,6 @@ This example creates a resolver that adds 10 to the given value (note the need t Custom resolvers support variadic argument lists in the form of a comma separated list of zero or more values. Whitespaces are stripped from both ends of each value ("foo,bar" is the same as "foo, bar "). You can use literal commas and spaces anywhere by escaping (:code:`\,` and :code:`\ `). - .. doctest:: >>> OmegaConf.register_resolver("concat", lambda x,y: x+y) @@ -369,17 +351,6 @@ You can use literal commas and spaces anywhere by escaping (:code:`\,` and :code >>> c.escape_whitespace 'Hello World' -You can take advantage of nested interpolations to perform operations over variables: - -.. doctest:: - - >>> OmegaConf.register_resolver("plus_int", - ... lambda x, y: int(x) + int(y)) - >>> c = OmegaConf.create({"a": 1, - ... "b": 2, - ... "a_plus_b": "${plus_int:${a},${b}}"}) - >>> c.a_plus_b - 3 Merging configurations diff --git a/news/308.feature b/news/308.feature deleted file mode 100644 index a11b098e2..000000000 --- a/news/308.feature +++ /dev/null @@ -1 +0,0 @@ -Add ability to nest interpolations, e.g. ${env:{$var}} or ${foo.${bar}.${baz}} diff --git a/omegaconf/__init__.py b/omegaconf/__init__.py index 055a6b329..10f26285c 100644 --- a/omegaconf/__init__.py +++ b/omegaconf/__init__.py @@ -1,7 +1,6 @@ from .base import Container, Node from .dictconfig import DictConfig from .errors import ( - InterpolationParseError, KeyValidationError, MissingMandatoryValue, ReadonlyConfigError, @@ -37,7 +36,6 @@ "ReadonlyConfigError", "UnsupportedValueType", "KeyValidationError", - "InterpolationParseError", "Container", "ListConfig", "DictConfig", diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py index 177e2d1f7..f839a2a98 100644 --- a/omegaconf/_utils.py +++ b/omegaconf/_utils.py @@ -316,15 +316,10 @@ def get_value_kind(value: Any, return_match_list: bool = False) -> Any: """ Determine the kind of a value Examples: - MANDATORY_MISSING : "???" + MANDATORY_MISSING : "??? VALUE : "10", "20", True, INTERPOLATION: "${foo}", "${foo.bar}" - STR_INTERPOLATION: "ftp://${host}/path", "${foo.${bar}}" - - Note in particular that in the current implementation, "${foo.${bar}}" is - identified as a string interpolation (`STR_INTERPOLATION`) while it is - actually a (nested) simple interpolation. This discrepancy w.r.t naming - conventions will be addressed in a future update. + STR_INTERPOLATION: "ftp://${host}/path" :param value: input string to classify :param return_match_list: True to return the match list as well diff --git a/omegaconf/base.py b/omegaconf/base.py index 826080b05..f539c5d92 100644 --- a/omegaconf/base.py +++ b/omegaconf/base.py @@ -5,20 +5,7 @@ from typing import Any, Dict, Iterator, Optional, Tuple, Type, Union from ._utils import ValueKind, _get_value, format_and_raise, get_value_kind -from .errors import ( - ConfigKeyError, - InterpolationParseError, - MissingMandatoryValue, - UnsupportedInterpolationType, -) - - -@dataclass -class InterpolationRange: - """Used to store start/stop indices of interpolations being evaluated""" - - start: int - stop: Optional[int] = None +from .errors import ConfigKeyError, MissingMandatoryValue, UnsupportedInterpolationType @dataclass @@ -371,6 +358,7 @@ def _resolve_interpolation( throw_on_missing: bool, throw_on_resolution_failure: bool, ) -> Any: + from .nodes import StringNode value_kind, match_list = get_value_kind(value=value, return_match_list=True) if value_kind not in (ValueKind.INTERPOLATION, ValueKind.STR_INTERPOLATION): @@ -389,124 +377,28 @@ def _resolve_interpolation( elif value_kind == ValueKind.STR_INTERPOLATION: value = _get_value(value) assert isinstance(value, str) - try: - # Note: `match_list` is ignored here as complex interpolations may be - # nested, the string will be re-parsed within `_evaluate_complex()`. - return self._evaluate_complex( - value, + orig = value + new = "" + last_index = 0 + for match in match_list: + new_val = self._resolve_simple_interpolation( key=key, + inter_type=match.group(1), + inter_key=match.group(2), throw_on_missing=throw_on_missing, throw_on_resolution_failure=throw_on_resolution_failure, ) - except InterpolationParseError as e: - if throw_on_resolution_failure: - self._format_and_raise(key=key, value=value, cause=e) - return None + # if failed to resolve, return None for the whole thing. + if new_val is None: + return None + new += orig[last_index : match.start(0)] + str(new_val) + last_index = match.end(0) + + new += orig[last_index:] + return StringNode(value=new, key=key) else: assert False - def _evaluate_simple( - self, - value: str, - key: Any, - throw_on_missing: bool, - throw_on_resolution_failure: bool, - ) -> Optional["Node"]: - """Evaluate a simple interpolation""" - value_kind, match_list = get_value_kind(value=value, return_match_list=True) - if value_kind == ValueKind.VALUE: - from .nodes import StringNode - - # False positive, this is not an interpolation after all! This may happen - # with strings like "${foo:abc=def}" that are not valid interpolations per - # current parsing rules in `get_value_kind()`. - return StringNode(key=key, value=value) - assert value_kind == ValueKind.INTERPOLATION and len(match_list) == 1, ( - value, - value_kind, - match_list, - ) - match = match_list[0] - return self._resolve_simple_interpolation( - inter_type=match.group(1), - inter_key=match.group(2), - key=key, - throw_on_missing=throw_on_missing, - throw_on_resolution_failure=throw_on_resolution_failure, - ) - - def _evaluate_complex( - self, - value: str, - key: Any, - throw_on_missing: bool, - throw_on_resolution_failure: bool, - ) -> Optional["Node"]: - """ - Evaluate a complex interpolation. - - A complex interpolation is more elaborate than "${a}" or "${a:b,c}", e.g.: - "Sentence: ${subject} {verb} ${object}" (string interpolation) - "${plus:${x},${y}} (calling a custom resolver on config variables) - "${${op}:${x},${y}} (same as previous but with a dynamic resolver) - "${${a}}" (fetching the key whose name is stored in `a`) - - The high-level logic consists in scanning the input string `value` from - left to right, keeping track of the tokens signaling the opening and closing - of each interpolation in the string. Whenever an interpolation is closed we - evaluate it and replace its definition with the result of this evaluation. - - As an example, consider the string `value` set to "${foo.${bar}}": - 1. We initialize our result with the original string: "${foo.${bar}}" - 2. Scanning `value` from left to right, the first interpolation to be - closed is "${bar}". We evaluate it: if it resolves to "baz", we update - the result string to "${foo.baz}". - 3. The next interpolation to be closed is now "${foo.baz}". We evaluate it: - if it resolves to "abc", we update the result accordingly to "abc". - 4. We are done scanning `value` => the final result is "abc". - """ - from .nodes import StringNode - - to_eval = [] # list of ongoing interpolations to be evaluated - # `result` will be updated iteratively from `value` to final result. - result = value - total_offset = 0 # keep track of offset between indices in `result` vs `value` - for idx, ch in enumerate(value): - if value[idx : idx + 2] == "${": - # New interpolation starting. - to_eval.append(InterpolationRange(start=idx - total_offset)) - elif ch == "}": - # Current interpolation is ending. - if not to_eval: - # This can happen if someone uses braces in their strings, which - # we should still allow (ex: "I_like_braces_{}_and_${liked}"). - continue - inter = to_eval.pop() - inter.stop = idx + 1 - total_offset - # Evaluate this interpolation. - val = self._evaluate_simple( - value=result[inter.start : inter.stop], - key=key, - throw_on_missing=throw_on_missing, - throw_on_resolution_failure=throw_on_resolution_failure, - ) - _cond_parse_error(val is not None, "unexpected error during parsing") - # If this interpolation covers the whole expression we are evaluating, - # then `val` is our final result. We should *not* cast it to string! - if inter.start == 0 and idx == len(value) - 1: - return val - # Update `result` with the evaluation of the interpolation. - val_str = str(val) - result = "".join( - [result[0 : inter.start], val_str, result[inter.stop :]] - ) - # Update offset based on difference between the length of the definition - # of the interpolation vs the length of its evaluation. - offset = inter.stop - inter.start - len(val_str) - total_offset += offset - _cond_parse_error(not to_eval, "syntax error - maybe no matching braces?") - return StringNode(value=result, key=key) - def _re_parent(self) -> None: from .dictconfig import DictConfig from .listconfig import ListConfig @@ -534,13 +426,3 @@ def _re_parent(self) -> None: def _has_ref_type(self) -> bool: return self._metadata.ref_type is not Any - -def _cond_parse_error(condition: Any, msg: str = "") -> None: - """ - Raise an `InterpolationParseError` if `condition` evaluates to `False`. - - `msg` is an optional message that may give additional information about - the error. - """ - if not condition: - raise InterpolationParseError(msg) diff --git a/omegaconf/errors.py b/omegaconf/errors.py index e51ff1ab6..fc2afd121 100644 --- a/omegaconf/errors.py +++ b/omegaconf/errors.py @@ -103,9 +103,3 @@ class ConfigValueError(OmegaConfBaseException, ValueError): """ Thrown from a config object when a regular access would have caused a ValueError. """ - - -class InterpolationParseError(OmegaConfBaseException): - """ - Thrown when unable to parse a complex interpolation. - """ diff --git a/tests/test_interpolation.py b/tests/test_interpolation.py index 49683e339..6be772dea 100644 --- a/tests/test_interpolation.py +++ b/tests/test_interpolation.py @@ -8,7 +8,6 @@ from omegaconf import ( DictConfig, IntegerNode, - InterpolationParseError, ListConfig, OmegaConf, Resolver, @@ -348,106 +347,11 @@ def test_incremental_dict_with_interpolation() -> None: ({"list": ["${ref}"], "ref": "bar"}, "list.0", "bar"), ], ) -def test_interpolations(cfg: Dict[str, Any], key: str, expected: Any) -> None: +def test_interpolations(cfg: DictConfig, key: str, expected: Any) -> None: c = OmegaConf.create(cfg) assert OmegaConf.select(c, key) == expected -@pytest.mark.parametrize( # type: ignore - "cfg,expected_dict", - [ - pytest.param( - """ - a: 1 - b: a - c: ${${b}} - """, - {"c": 1}, - id="basic_nesting", - ), - pytest.param( - """ - a: OMEGACONF - b: NESTED_INTERPOLATIONS_TEST - c: ${env:${a}_${b}} - """, - {"c": "test123"}, - id="nesting_with_key", - ), - pytest.param( - """ - x: 1 - y: 2 - op: plus - z: ${plus:${x},${y}} - t: ${${op}:${x},${y}} - """, - {"z": 3, "t": 3}, - id="nesting_with_resolver", - ), - pytest.param( - """ - a: - b: 1 - c: 2 - d: ${a.b} - b: c - c: ${a.${b}} - d: ${${b}} - e: .d - f: ${a${e}} - """, - {"c": 2, "d": 2, "f": 1}, - id="member_access", - ), - pytest.param( - """ - a: def - b: abc_{${a}} - """, - {"b": "abc_{def}"}, - id="braces_in_string", - ), - pytest.param( - """ - a: A - b: ${env:x=A} - c: ${env:x=${a}} - """, - # This behavior may be a bit surprising but it is how it works now. If - # it changes in the future we should ensure that both `b` and `c` yield - # the same result. - {"b": "${env:x=A}", "c": "${env:x=A}"}, - id="illegal_character_in_interpolation", - ), - ], -) -def test_nested_interpolations(cfg: str, expected_dict: Dict[str, Any]) -> None: - os.environ["OMEGACONF_NESTED_INTERPOLATIONS_TEST"] = "test123" - OmegaConf.register_resolver("plus", lambda x, y: int(x) + int(y)) - try: - c = OmegaConf.create(cfg) - for key, expected in expected_dict.items(): - assert OmegaConf.select(c, key) == expected - finally: - OmegaConf.clear_resolvers() - - -@pytest.mark.parametrize( # type: ignore - "cfg,key", - [ - # All these examples have non-matching braces. - pytest.param({"a": "PATH", "b": "${env:${a}"}, "b", id="env_not_closed"), - pytest.param({"a": 1, "b": 2, "c": "${a ${b}"}, "c", id="a_not_closed"), - pytest.param({"a": 1, "b": 2, "c": "${a} ${b"}, "c", id="b_not_closed"), - ], -) -def test_nested_interpolation_errors(cfg: Dict[str, Any], key: str) -> None: - c = OmegaConf.create(cfg) - with pytest.raises(InterpolationParseError): - OmegaConf.select(c, key) - - def test_interpolation_with_missing() -> None: cfg = OmegaConf.create({"out_file": "${x.name}.txt", "x": {"name": "???"}}) assert OmegaConf.is_missing(cfg, "out_file")