Skip to content

Commit

Permalink
Add tests and news fragment
Browse files Browse the repository at this point in the history
Fixes omry#572
  • Loading branch information
odelalleau committed Feb 26, 2021
1 parent e262502 commit 9ecac27
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 3 deletions.
2 changes: 2 additions & 0 deletions news/572.api_change
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Implicitly empty resolver arguments (e.g., `${foo:a,}`) are deprecated in favor of explicit quoted strings (e.g., `${foo:a,""}`)

5 changes: 3 additions & 2 deletions omegaconf/grammar_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,13 @@ def visitSequence(
# (element (COMMA element?)*) | (COMMA element?)+
assert ctx.getChildCount() >= 1

# DEPRECATED: remove in 2.2 (revert #571)
def empty_str_warning() -> None:
txt = ctx.getText()
warnings.warn(
f"In the sequence `{txt}` some elements are missing: please replace "
f"them with empty strings (\"\" or '') since this will not be supported "
f"anymore in the future.",
f"them with empty quoted strings. "
f"See https://github.com/omry/omegaconf/issues/572 for details.",
category=UserWarning,
)

Expand Down
27 changes: 26 additions & 1 deletion tests/test_grammar.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import math
import re
from typing import Any, Callable, List, Optional, Tuple

import antlr4
from pytest import mark, param, raises
from pytest import mark, param, raises, warns

from omegaconf import (
DictConfig,
Expand Down Expand Up @@ -369,6 +370,30 @@ def test_config_value(
parse_tree, expected_visit = self._parse("configValue", definition, expected)
self._visit_with_config(parse_tree, expected_visit)

@parametrize_from(
[
("trailing_comma", "${test:a,b,}", ["a", "b", ""]),
("empty_middle", "${test:a,,b}", ["a", "", "b"]),
("empty_first", "${test:,a,b}", ["", "a", "b"]),
("single_comma", "${test:,}", ["", ""]),
(
"mixed_with_ws",
"${test: ,a,b,\t,,c, \t \t ,d,, \t}",
["", "a", "b", "", "", "c", "", "d", "", ""],
),
]
)
def test_deprecated_empty_args(
self, restore_resolvers: Any, definition: str, expected: Any
) -> None:
OmegaConf.register_new_resolver("test", self._resolver_test)

parse_tree, expected_visit = self._parse("singleElement", definition, expected)
with warns(
UserWarning, match=re.escape("https://github.com/omry/omegaconf/issues/572")
):
self._visit_with_config(parse_tree, expected_visit)

def _check_is_same_type(self, value: Any, expected: Any) -> None:
"""
Helper function to validate that types of `value` and `expected are the same.
Expand Down

0 comments on commit 9ecac27

Please sign in to comment.