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.

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

feat: set flag readonly recursively #1124

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

MatteoVoges
Copy link
Contributor

Motivation

I want a function that sets a value for a flag (atm readonly, but can be extended for all / generic flags) recursively.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Tests will be added if the basic structure is confirmed.

Fixes

Fixes #1123

@MatteoVoges
Copy link
Contributor Author

@odelalleau with this I get:

from omegaconf import OmegaConf

config = OmegaConf.create({"a": {"b": 1}})

OmegaConf.set_readonly(config.a, True)
OmegaConf.set_readonly_recursively(config, False)
print(OmegaConf.is_readonly(config.a))
# >>> False

@MatteoVoges MatteoVoges changed the title feat: set flag readonly recursively feat: set flag readonly recursively Sep 20, 2023
@odelalleau
Copy link
Collaborator

Thanks! I would suggest to do it a bit differently to have a more generic implementation:

  • Add a recursive boolean argument to _set_flag()
  • Also add it to set_struct() and set_readonly() (instead of adding a different function specific to readonly)

@odelalleau
Copy link
Collaborator

Thanks for the update -- overall looks good to me, but I won't have time to look at it in details until next week. We'll need to add a news fragment as well and update the documentation.

@MatteoVoges MatteoVoges force-pushed the feat/set-flag-recursively branch from a4995a7 to c64a9db Compare September 26, 2023 09:05
@indigoviolet
Copy link

@omry @odelalleau this is a blocker for #1102, would be great to get it in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistencies in recursive behavior in set_readonly()
3 participants