omry / omegaconf

Flexible Python configuration system. The last one you will ever need.
BSD 3-Clause "New" or "Revised" License
1.95k stars 107 forks source link

Interpolations that resolve to missing value `???` don't get passed to resolvers #1130

Open MatteoVoges opened 12 months ago

MatteoVoges commented 12 months ago

Is your feature request related to a problem? Please describe. I have some compare-resolver that checks if some values are equal or not. When I tried to compare against the MISSING_VALUE ???, the resolver does not get called.

Here's a simpler repro-example with a resolver, that just prints its input:

from omegaconf import OmegaConf

OmegaConf.register_new_resolver("print", print) # the default print function

config = OmegaConf.create({"a": "${print:${b}}", "b": "???"})

OmegaConf.resolve(config)
# prints nothing, but should print "???"

print(config)
# >>> {'a': '???', 'b': '???'}

I think it's a general problem with the missing value. The value for a at the end should be None and not ???. I can follow the reasons why it's implemented in that way, that if some sub-interpolations are ???, the whole interpolation gets evaluated as ???.

Describe the solution you'd like Ideally such an interpolation should be treated as a normal value like the string ??? unless you call some function with throw_on_missing = true.

Thanks in advance for your help!

odelalleau commented 12 months ago

This is by design and unlikely to change. Allowing missing values to be passed as input to resolvers would force all resolvers to implement some handling of missing values, which we definitely don't want. What might be possible is adding a flag to register_new_resolver() to indicate that it's ok for the resolver to take a missing value as input, but I haven't looked at this code recently so I'm not sure how easy it'd be to add.

By the way this makes me realize there is an issue: I was expecting OmegaConf.resolve(config) to crash, but it didn't. This is annoying because after resolution OmegaConf.is_missing(config, "a") is True, while it is False before => this is definitely a discrepancy that should be looked into.

MatteoVoges commented 11 months ago

What might be possible is adding a flag to register_new_resolver() to indicate that it's ok for the resolver to take a missing value as input

Is this functionality actually located on the resolver side or in the resolve logic? If so, is there like a specific if-clause that breaks for the missing value? Unfortunately I don't have time at the moment to look deeper into it...

odelalleau commented 11 months ago

Is this functionality actually located on the resolver side or in the resolve logic? If so, is there like a specific if-clause that breaks for the missing value? Unfortunately I don't have time at the moment to look deeper into it...

The error is raised during resolution, at https://github.com/omry/omegaconf/blob/7dae67e4a3869b584fd6d6408b2f916c176755db/omegaconf/base.py#L670 Currently it's not caught so it just makes the program crash. If we wanted to allow a resolver to process a missing value, I think we'd need to catch this exception at https://github.com/omry/omegaconf/blob/97c0a47582699cc544ef91f73dbadf210b6a4662/omegaconf/grammar_visitor.py#L174 and forward the information about the missing value to the resolver callback.

Note that looking at the code I noticed there's a workaround: use ${oc.select:node, default_if_missing} instead of ${node} => you can pick a default value that your resolver recognizes as indicating a missing value.