-
Notifications
You must be signed in to change notification settings - Fork 118
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.
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
Add is_unresolved
to OmegaConf
#1081
Comments
What is the difference between your new proposed function and is_interpolation ? |
I am starting to see what you are asking for. cfg = OmegaConf.create({"a": "${b}", "b" : 10})
OmegaConf.resolve(cfg)
# resolved?
cfg.a = "${b}"
# not resolved again? Feel free to add some more color to what is the use case. At this point I don't think this something worth adding @Jasha10, @odelalleau, @shagunsodhani: any comments? |
Like I said, if OmegaConf can return the is_resolved status, then I can choose to use a wrapper function to access values instead of using the dot notation or accessor functions. get_val = lambda conf, var: conf._get_node(var)._value()
if not OmegaConf.is_resolved(config): OmegaConf.resolve(config)
# now we use only get_val and nothing else
print(config.debug) # Don't allow during code reviews
print(get_val(config, "debug")) # Safe for leaf nodes It's not perfect. A slightly better way would be to add an option to the getter so we don't have to rely on a In our usecase, we don't control the loading of the config, that is handled by either a test stub or a library. And depending on what the test is testing, we want to resolve or not resolve the config (in our test stub) to keep a consistent behavior |
I think the main issue here is that if an interpolation returns a string that looks like an interpolation, then the behavior of the original config and the resolved config aren't the same anymore. This is a problem because in general we expect to be able to use Two potential solutions:
I think option (2) should be simpler to implement, and would provide a more straightforward user experience in most cases as well as better backward compatibility (not having to worry about a new config flag). The only thing that I can see as being potentially confusing is the fact that after resolution, we may still have keys that are considered as interpolations, because escaped interpolations are a special kind of interpolation. So someone using |
I think it makes sense to try and enforce this invariant. Some users might complain that they want to break this invariant, e.g. using environment variables to do tricky things (as @kunaltyagi hinted in the screenshot above): >>> os.environ["FOO"] = "${b}"
>>> cfg = OmegaConf.create("{a: '${oc.env:FOO}', b: 123}")
>>> cfg.a
'${b}'
>>> OmegaConf.resolve(cfg)
>>> cfg.a
123 Brainstorming - here's an idea that may or may not make sense: This option should enforce the invariant on |
Can both of these be done? Maybe we can have an API something like the following OmegaConf.resolve(cfg, depth:int|None)
# depth=0...INT_MAX: No future resolutions: 0: infinite recursion, 1...INT_MAX for recursion depth
# depth = None, same as calling depth=1 and then unlock_interpolation
OmegaConf.lock_interpolation(cfg) # all ${x} now treated as strings
OmegaConf.unlock_interpolation(cfg) # back to default behavior |
The issue with making interpolation resolution recursive is that it wouldn't be possible to have
To be clear, there would be no need to un-escape strings in Option 2. You could have |
Good point.
This would require the node to own extra metadata for keeping track of whether the string needs to be unescaped when |
No, this mechanism already exists and is part of the resolution of interpolations. |
|
>>> os.environ["FOO"] = "${b}"
>>> cfg = OmegaConf.create("{a: '${oc.env:FOO}', b: 123}")
>>> cfg.a
'${b}'
>>> OmegaConf.resolve(cfg)
>>> cfg.a
123
Re: point 1, that's a consequence of the fact that the public api always calls resolve while accessing |
I have never seen anyone using interpolations in environment variables before. import os
from typing import Any, Optional
from omegaconf import OmegaConf, Node, AnyNode
from omegaconf._utils import _DEFAULT_MARKER_
def myenv(key: str, _node_: Node, default: Any = _DEFAULT_MARKER_) -> Optional[str]:
try:
env = os.environ[key]
# tmp node with the same parent
tmp = AnyNode(env, parent=_node_._parent)
return tmp._dereference_node()._value()
except KeyError:
if default is not _DEFAULT_MARKER_:
return str(default) if default is not None else None
else:
raise KeyError(f"Environment variable '{key}' not found")
OmegaConf.register_new_resolver("myenv", myenv)
os.environ["FOO"] = "${b}"
cfg = OmegaConf.create({
"a": '${myenv:FOO}',
"b": 123
})
print(cfg.a) |
I feel like my solution is addressing your problem. Feel free to followup here further. |
Is your feature request related to a problem? Please describe.
I'd like to know when the value would be an indirect call and when it's a direct input
Describe the solution you'd like
There are other helper functions like
is_missing
andis_unresolved
would be able to allow custom logic around detecting the source of the valueDescribe alternatives you've considered
None
The text was updated successfully, but these errors were encountered: