Improve error message when initializing from a Structured Config with incorrect type #559
Conversation
Jasha10
force-pushed
the
improve-structured-conf-err-reporting2
branch
from
a3a2a0e to
11dd051
Compare
|
Can you add a test when the error is in a deeper level to see the resulting error message is reasonable? maybe something like: create=lambda: OmegaConf.structured(
ConcretePlugin(params=ConcretePlugin.FoobarParams(foo="x")) # type: ignore
), |
Jasha10
force-pushed
the
improve-structured-conf-err-reporting2
branch
from
3f40dbb to
2610a53
Compare
omegaconf/_utils.py
Outdated
| parent=dummy_parent, | ||
| ) | ||
| except ValidationError as ex: | ||
| dummy_parent._format_and_raise(key=name, value=value, cause=ex) |
There was a problem hiding this comment.
can you try to use format_and_raise() directly instead of through the dummy parent?
it can probably make things a bit cleaner when testing this.
There was a problem hiding this comment.
In f2a1bfd I've switched from dummy_parent._format_and_raise(...) to format_and_raise(node=None, ...).
The disadvantage of this change is that we no longer have object_type information in the error message (as dummy_parent was providing the object_type).
There was a problem hiding this comment.
Besides the object_type issue, the error messages are also not reporting full_key accurately. I think that reporting on full_key would be easiest if we pass a parent_node into the get_structured_config_data function (so that full_key could be computed using the parent config). This change would require some plumbing of the methods that call get_structured_config_data. I can give it a try if you're interested...
There was a problem hiding this comment.
I think this will trivially easier once we have the structured_ir diff baked and ready.
for now this is good enough.
omry
changed the title
|
Can you add a news fragment file? feel free to squash and merge in once you do. |
… incorrect type (omry#559) * test error message * use format_and_raise() directly * add 435.bugfix news fragment
Closes #435.
Based on http://paste.ubuntu.com/p/d7zKBpG9rg/