Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hydra] Improve error message in parse_overrides #3022

Merged
merged 12 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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---"
)
Loading