Skip to content

Commit c93d809

Browse files
committed
Fail composition when a dangling override is rescued by an append
A Defaults List override with no earlier Group Default to override correctly fails on its own, but a matching command line append made the composition succeed and silently replaced the appended value with the dangling override's. Stop registering a defaults list's own override when an externally appended group default of the same key is resolved. The appended entry now composes without consulting the dangling override, which is then reported unused, so the composition fails exactly as it does without the append. Command line overrides of appended groups are registered before traversal and still apply. Add regression coverage for the dangling override with and without the append. Fixes #3318
1 parent ee7d3e7 commit c93d809

3 files changed

Lines changed: 18 additions & 19 deletions

File tree

hydra/_internal/defaults_list.py

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -574,16 +574,6 @@ def _create_defaults_tree_impl(
574574

575575
_update_overrides(defaults_list, overrides, parent, interpolated_subtree)
576576

577-
# An externally appended group default is the one case where a group default
578-
# may legally follow an override of the same group in a single defaults list.
579-
# The reverse traversal below visits the appended entry before that override,
580-
# so map each key to the list's last override for it, allowing the appended
581-
# entry to register its override before resolving.
582-
list_overrides: Dict[str, GroupDefault] = {}
583-
for d in defaults_list:
584-
if isinstance(d, GroupDefault) and d.is_override():
585-
list_overrides[d.get_override_key()] = d
586-
587577
def add_child(
588578
child_list: List[Union[InputDefault, DefaultsTreeNode]],
589579
new_root_: DefaultsTreeNode,
@@ -613,11 +603,6 @@ def add_child(
613603

614604
d.update_parent(parent.get_group_path(), parent.get_final_package())
615605

616-
if isinstance(d, GroupDefault) and d.is_external_append():
617-
pending = list_overrides.get(d.get_override_key())
618-
if pending is not None:
619-
overrides.add_override(parent.get_config_path(), pending)
620-
621606
if overrides.is_overridden(d):
622607
assert isinstance(d, GroupDefault)
623608
overrides.override_default_option(d)

news/3318.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
A Defaults List override that does not target an earlier Group Default now fails to compose even when a command line append introduces a matching group, instead of silently discarding the appended value.

tests/defaults_list/test_defaults_tree.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,11 +215,15 @@ def test_simple_defaults_tree_cases(
215215
param(
216216
"group_override_only",
217217
["+group1=file1"],
218-
DefaultsTreeNode(
219-
node=ConfigDefault(path="group_override_only"),
220-
children=[GroupDefault(group="group1", value="file2")],
218+
raises(
219+
ConfigCompositionException,
220+
match=re.escape(
221+
dedent("""\
222+
In 'group_override_only': Could not override 'group1'.
223+
Did you mean to override group1?""")
224+
),
221225
),
222-
id="group_override_only:append_overridden_group",
226+
id="group_override_only:append_does_not_rescue_dangling_override",
223227
),
224228
],
225229
)
@@ -2625,6 +2629,15 @@ def test_missing_config_errors(
26252629
),
26262630
id="error_invalid_override",
26272631
),
2632+
param(
2633+
"group_override_only",
2634+
[],
2635+
raises(
2636+
ConfigCompositionException,
2637+
match="Could not override 'group1'. No match in the defaults list.",
2638+
),
2639+
id="dangling_override_without_append",
2640+
),
26282641
param(
26292642
"group_default",
26302643
["group1@foo=file1"],

0 commit comments

Comments
 (0)