Better and more consistent handling of the "missing" status

[odelalleau]

Describe the bug

Currently the handling of a missing node in interpolations can lead to surprising behavior like:

cfg = OmegaConf.create({"x": "${missing}", "y": "abc ${missing}", "missing": "???"})
print(f'{cfg._get_node("x")._is_missing() = }')
print(f'{cfg._get_node("x")._dereference_node() = }')
print(f'{cfg._get_node("y")._is_missing() = }')
print(f'{cfg._get_node("y")._dereference_node() = }')

which prints:

cfg._get_node("x")._is_missing() = True
cfg._get_node("x")._dereference_node() = '???'
cfg._get_node("y")._is_missing() = False
cfg._get_node("y")._dereference_node() = 'abc ???'

The two potentially surprising things are:

1. The fact that x is considered as missing while y isn't (while both refer to a missing node)
2. The fact that y is happily dereferenced into a StringNode containing the string "abc ???"

Expected behavior

I propose the following behavior:

1. A node is never missing if it is an interpolation (even if it interpolates into a missing node). This is consistent with the spirit of Behavior change: Merging a MISSING node should not change target #462, where we want "missing" to mean only being set to "???"
2. Dereferencing a node containing an interpolation to a missing node should return None (unless throw_on_missing throw_on_resolution_failure is True, in which case it should raise the InterpolationToMissingValueError exception)

(Edit: I replaced throw_on_missing with throw_on_resolution_failure in point 2 because throw_on_missing when dereferencing a node should only refer to the status of the node being referenced, not to what happens when trying to resolve interpolations, while throw_on_resolution_failure relates to errors encountered when resolving interpolations)

[omry]

The only surprise to me is cfg._get_node("y")._dereference_node() = 'abc ???'.
This should raise an exception while resolving the string interpolation.

This behavior is driving the contains API behavior for missing fields:

"missing" in cfg`

which returns False.
In a previous version string interpolation using a missing field was also considered missing, but I changed that at some point.

I am open to changing this behavior but we should consider it carefully and check in downstream apps (at least in Hydra).

[odelalleau]

In a previous version string interpolation using a missing field was also considered missing, but I changed that at some point.

Yes, that was in #462, and I think it makes sense to also extend the same logic (not being missing) to any interpolation using a missing field, not just string interpolations, including in particular:

• Nested interpolations like ${env:${missing}} or ${foo.${missing}}
• Direct interpolations ${missing}

The last one is probably the most controversial, but I think it is what makes most sense to have simple and consistent semantics for what "missing" means ("has the key been set in the config?"). I ran Hydra's test suite with this change btw, and nothing broke.

[omry]

The origin of this behavior predated the OmegaConf.is_missing() public API.
key in cfg was used as a proxy to detect if a field is missing.

Now that we have a formal API to check if a field is missing I support this change.