Skip to content

Commit

Permalink
[Hydra] Improve error message in parse_overrides (#3022)
Browse files Browse the repository at this point in the history
* [Hydra] Improve error message in parse_overrides

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test

* Fix test
  • Loading branch information
wu0607 authored Feb 5, 2025
1 parent ea7a86c commit 618ab4b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 23 deletions.
5 changes: 3 additions & 2 deletions hydra/core/override_parser/overrides_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def parse_override(self, s: str) -> Override:

def parse_overrides(self, overrides: List[str]) -> List[Override]:
ret: List[Override] = []
for override in overrides:
for idx, override in enumerate(overrides):
try:
parsed = self.parse_rule(override, "override")
except HydraException as e:
Expand All @@ -98,7 +98,8 @@ def parse_overrides(self, overrides: List[str]) -> List[Override]:
msg = f"Error parsing override '{override}'" f"\n{e}"
raise OverrideParseException(
override=override,
message=f"{msg}"
message=f"Error when parsing index: {idx}, string: {override} out of {overrides}."
f"\n{msg}"
f"\nSee https://hydra.cc/docs/1.2/advanced/override_grammar/basic for details",
) from e.__cause__
assert isinstance(parsed, Override)
Expand Down
66 changes: 45 additions & 21 deletions tests/test_hydra_cli_errors.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved
import re
from pathlib import Path
from typing import Any
from typing import Any, List

from pytest import mark, param

Expand All @@ -15,78 +14,103 @@


@mark.parametrize(
"override,expected",
"override,expected_substrings",
[
param(
"+key=int(",
"no viable alternative at input 'int('",
[
"Error when parsing index: 1, string: +key=int( out of [",
"no viable alternative at input 'int('",
],
id="parse_error_in_function",
),
param(
"+key=sort()",
"""Error parsing override '+key=sort()'
[
"""Error when parsing index: 1, string: +key=sort() out of [""",
"""Error parsing override '+key=sort()'
ValueError while evaluating 'sort()': empty sort input""",
],
id="empty_sort",
),
param(
"key=sort(interval(1,10))",
"""Error parsing override 'key=sort(interval(1,10))'
[
"""Error when parsing index: 1, string: key=sort(interval(1,10)) out of [""",
"""Error parsing override 'key=sort(interval(1,10))'
TypeError while evaluating 'sort(interval(1,10))': mismatch type argument args[0]""",
],
id="sort_interval",
),
param(
"+key=choice()",
"""Error parsing override '+key=choice()'
[
"""Error when parsing index: 1, string: +key=choice() out of [""",
"""Error parsing override '+key=choice()'
ValueError while evaluating 'choice()': empty choice is not legal""",
],
id="empty choice",
),
param(
"+key=extend_list(1, 2, 3)",
"""Error parsing override '+key=extend_list(1, 2, 3)'
[
"""Error when parsing index: 1, string: +key=extend_list(1, 2, 3) out of [""",
"""Error parsing override '+key=extend_list(1, 2, 3)'
Trying to use override symbols when extending a list""",
],
id="plus key extend_list",
),
param(
"key={inner_key=extend_list(1, 2, 3)}",
"no viable alternative at input '{inner_key='",
[
"""Error when parsing index: 1, string: key={inner_key=extend_list(1, 2, 3)} out of [""",
"no viable alternative at input '{inner_key='",
],
id="embedded extend_list",
),
param(
["+key=choice(choice(a,b))", "-m"],
"""Error parsing override '+key=choice(choice(a,b))'
[
"""Error when parsing index: 1, string: +key=choice(choice(a,b)) out of [""",
"""Error parsing override '+key=choice(choice(a,b))'
ValueError while evaluating 'choice(choice(a,b))': nesting choices is not supported
See https://hydra.cc/docs/1.2/advanced/override_grammar/basic for details
Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace.
""",
],
id="empty choice",
),
param(
"--config-dir=/dir/not/found",
f"""Additional config directory '{Path('/dir/not/found').absolute()}' not found
[
f"""Additional config directory '{Path('/dir/not/found').absolute()}' not found
Set the environment variable HYDRA_FULL_ERROR=1 for a complete stack trace.
""",
"""
],
id="config_dir_not_found",
),
],
)
def test_cli_error(tmpdir: Any, monkeypatch: Any, override: Any, expected: str) -> None:
def test_cli_error(
tmpdir: Any,
monkeypatch: Any,
override: Any,
expected_substrings: List[str],
) -> None:
monkeypatch.chdir("tests/test_apps/app_without_config/")
if isinstance(override, str):
override = [override]
cmd = ["my_app.py", "hydra.sweep.dir=" + str(tmpdir)] + override
ret = normalize_newlines(run_with_error(cmd))
assert (
re.search("^" + re.escape(normalize_newlines(expected.strip())), ret)
is not None
), (
missing_substrings = [s for s in expected_substrings if s.strip() not in ret]
assert not missing_substrings, (
f"Result:"
f"\n---"
f"\n{ret}"
f"\n---"
f"\nDid not match expected:"
f"\n---"
f"\n{expected}"
f"\n---"
f"\nDoes not contain the following expected substrings:"
+ "\n---\n".join(f"{s}" for s in missing_substrings)
+ "\n---"
)

0 comments on commit 618ab4b

Please sign in to comment.