-
Notifications
You must be signed in to change notification settings - Fork 116
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
Inconsistencies in recursive behavior in set_readonly()
#1123
Comments
Although I agree this can be confusing, I'm not sure this is actually a bug. The way flags work in OmegaConf is that a node inherits its parent's flags except for those that are explicitly set for this node. See code at: Line 200 in 7dae67e
The order in which flags are set doesn't matter. Whether you do OmegaConf.set_readonly(config, False)
OmegaConf.set_readonly(config.a, True) or OmegaConf.set_readonly(config.a, True)
OmegaConf.set_readonly(config, False) you will always have I think it'd make sense to add an option to override the child nodes' flags, but in the meantime, my suggestion would be that you write your own function that iterates over children to achieve this. |
Thanks for the explanation. I just thought that the behavior should be the same, regardless if the value of the flag gets set to |
It should indeed be the same.
I agree it'd be good to also have a way to clear flags recursively. |
Ahh you're right. I forgot for a moment, that directly set flags have higher priority than the parent flags 🤦♂️ . |
IIRC You can reset the value of a node by setting the flag value on that node to None. |
I would be happy with both options. I just need a config with no flags, therefor its not important, if the value is |
Yeah I think a recursive |
To be honest, I think set_flag with a recursive flag is more confusing. |
Describe the bug
There is unexpected behavior for different types of recursive flag passing when using
OmegaConf.set_readonly()
To Reproduce
Here we see in the first example, that when applying
readonly
to a config, that subconfigs get recursively updated with the correct state.In the second example we apply
readonly
to a subconfig, and the parent node doesn't get updated, but when settingreadonly
to false in the parent node, the children don't get recursively updated, which is wrong.Expected behavior
The child nodes should get correctly updated.
Additional context
The text was updated successfully, but these errors were encountered: