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.

Sign up for GitHub

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

Jump to bottom

Ensure all exceptions raised while resolving interpolations derive from a common class #561

Closed
odelalleau opened this issue Feb 24, 2021 · 2 comments · Fixed by #545
Closed

Ensure all exceptions raised while resolving interpolations derive from a common class #561

odelalleau opened this issue Feb 24, 2021 · 2 comments · Fixed by #545
Assignees
@odelalleau
Milestone
OmegaConf 2.1

Comments

@odelalleau
Copy link
Collaborator

odelalleau commented Feb 24, 2021 •
edited
Loading

Is your feature request related to a problem? Please describe.

Currently, many types of exceptions can be raised when an issue occurs during interpolation resolution, which makes it difficult to separate interpolation errors from other exceptions, and to identify what went wrong during resolution.

Describe the solution you'd like

Have all interpolation-related exceptions be either InterpolationResolutionError or a subclass of it. The subclasses would be:

  • UnsupportedInterpolationType (trying to call a custom resolver that does not exist)
  • InterpolationKeyError (trying to interpolate a node that does not exist)
  • InterpolationToMissingValueError (trying to interpolate a node that is missing, i.e., set to "???")

Additional context
This is addressed by #545. Creating this issue to keep track of the changes.
@odelalleau odelalleau mentioned this issue Feb 24, 2021
Merged
@omry
Copy link
Owner

omry commented Feb 24, 2021

Since this (and other issues are for 2.1, can you set the milestone for it and others accordingly)?

@odelalleau odelalleau added this to the OmegaConf 2.1 milestone Feb 24, 2021
@odelalleau odelalleau self-assigned this Feb 24, 2021
@odelalleau
Copy link
Collaborator Author

Since this (and other issues are for 2.1, can you set the milestone for it and others accordingly)?

All done!

@omry omry closed this as completed in #545 Mar 3, 2021
omry pushed a commit that referenced this issue Mar 3, 2021
@odelalleau
Improve handling of interpolations pointing to missing nodes (#545)
cb5e556 
* Interpolations are never considered to be missing anymore, even if
  they point to a missing node

* When resolving an expression containing an interpolation pointing to a
  missing node, an `InterpolationToMissingValueError` exception is raised

* When resolving an expression containing an interpolation pointing to a
  node that does not exist, an `InterpolationKeyError` exception is raised

* `key in cfg` returns True whenever `key` is an interpolation, even if it cannot be resolved (including interpolations to missing nodes)

* `get()` and `pop()` no longer return the default value in case of interpolation resolution failure (same thing for `OmegaConf.select()`)

* If `throw_on_resolution_failure` is False, then resolving an
  interpolation resulting in a resolution failure always leads to the
  result being `None` (instead of potentially being an expression computed
  from `None`)

Fixes #543
Fixes #561
Fixes #562
Fixes #565
odelalleau added a commit to odelalleau/omegaconf that referenced this issue Mar 10, 2021
@odelalleau
Improve handling of interpolations pointing to missing nodes (omry#545)
0329ef0 
* Interpolations are never considered to be missing anymore, even if
  they point to a missing node

* When resolving an expression containing an interpolation pointing to a
  missing node, an `InterpolationToMissingValueError` exception is raised

* When resolving an expression containing an interpolation pointing to a
  node that does not exist, an `InterpolationKeyError` exception is raised

* `key in cfg` returns True whenever `key` is an interpolation, even if it cannot be resolved (including interpolations to missing nodes)

* `get()` and `pop()` no longer return the default value in case of interpolation resolution failure (same thing for `OmegaConf.select()`)

* If `throw_on_resolution_failure` is False, then resolving an
  interpolation resulting in a resolution failure always leads to the
  result being `None` (instead of potentially being an expression computed
  from `None`)

Fixes omry#543
Fixes omry#561
Fixes omry#562
Fixes omry#565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@odelalleau odelalleau

Labels
None yet
Projects
None yet
Milestone
OmegaConf 2.1
Development

Successfully merging a pull request may close this issue.

Improve handling of interpolations pointing to missing nodes odelalleau/omegaconf
2 participants
@omry @odelalleau