Error message improvement for OmegaConf.structured creation #557
Conversation
Jasha10
changed the title
| except ValidationError as e: | ||
| format_and_raise(node=parent, key=key, value=value, msg=str(e), cause=e) |
There was a problem hiding this comment.
The main change of this PR is to wrap the body of OmegaConf._node_wrap in a try/catch block, so that more information can be given to the format_and_raise function. The goal is to close #435.
I wasn't sure if passing node=parent to format_and_raise above is the right decision. I tried passing node=None, which resulted in a lot more errors.
There was a problem hiding this comment.
After further review recalled that, in the case of structured config creation, the parent is a dummy_parent created by the get_dataclass_data function.
| # structured typecheck | ||
| pytest.param( | ||
| Expected( | ||
| create=lambda: None, | ||
| op=lambda cfg: OmegaConf.structured(User(age="foo")), # type: ignore | ||
| exception_type=ValidationError, | ||
| msg="Value 'foo' could not be converted to Integer", | ||
| object_type=dict, | ||
| key="age", | ||
| low_level=True, | ||
| ), | ||
| id="structured:creation-with-incorrect-key-type", | ||
| ), |
There was a problem hiding this comment.
This is the test case motivated by #435.
The object_type=dict comes from passing node=parent to the format_and_raise function, I think.
| object_type_str=None, | ||
| ref_type_str=None, | ||
| object_type=dict, | ||
| key="foo", | ||
| low_level=True, |
There was a problem hiding this comment.
Made this change to get the tests to pass. I will review to see if there is a better way to achieve the improved error message, without having to change the above test case.
| object_type=None, | ||
| msg="Non optional field cannot be assigned None", | ||
| object_type_str="NotOptionalInt", | ||
| ref_type_str=None, | ||
| object_type=dict, | ||
| key="foo", | ||
| low_level=True, |
Closes #435