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

Escaped resolver modifying _root_ doesn't work #1120

Open
4 tasks done
MatteoVoges opened this issue Aug 30, 2023 · 2 comments
Open
4 tasks done

Escaped resolver modifying _root_ doesn't work #1120

MatteoVoges opened this issue Aug 30, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@MatteoVoges
Copy link
Contributor

Describe the bug
I have a resolver, that modifies _root_. When the resolver is unescaped, it works fine, but if it's escaped, it doesn't behave as expected.
This could be interesting for /related to #1113 @odelalleau

To Reproduce
Please provide a minimal repro.

from omegaconf.omegaconf import OmegaConf as oc

def modify_root(key, value, _root_):
    _root_[key] = value
    return "done"

test = {
    "a": "a",
    "b": "b",
    "modify_a": "${modify_root:a,MODIFIED}",
    "modify_b": "\${modify_root:b,MODIFIED}"
}

oc.register_new_resolver("modify_root", modify_root)

config = oc.create(test)
oc.resolve(config)
obj = oc.to_object(config) # resolving second time
print(obj)
# >>> {'a': 'MODIFIED', 'b': 'b', 'modify_a': 'done', 'modify_b': 'done'}

We see, that both times the resolver got executed, but only the unescaped one could modify its key (a).

Expected behavior
I would expect, that after resolving twice, the key b gets MODIFIED too.

Additional context

  • OmegaConf version: master (2.4.0.1.dev)
  • Python version: 3.10.12
  • Operating system: Linux Mint
  • Please provide a minimal repro
@MatteoVoges MatteoVoges added the bug Something isn't working label Aug 30, 2023
@odelalleau
Copy link
Collaborator

odelalleau commented Aug 30, 2023

Yes it's related to #1113. I knew I had to look into the behavior of to_object() / to_container() but didn't have time for it yet.

In the meantime, the workaround is to manually call oc.resolve(config) before oc.to_object(config).

Edit: actually since to_object() is also trying to resolve, I think the workaround would rather be

oc.resolve(config)
obj = oc.to_container(
    cfg=config,
    resolve=False,
    throw_on_missing=True,
    enum_to_str=False,
    structured_config_mode=SCMode.INSTANTIATE,
)

(if you don't want to modify config, you'd need to deepcopy it first)

@MatteoVoges
Copy link
Contributor Author

MatteoVoges commented Aug 31, 2023

I don't see why the resolving behaves different in oc.resolve() and oc.to_object()

os.resolve() works in-place by replacing interpolations with their values. oc.to_object() creates a new config with new nodes and this is using a different code path where the new escape_interpolation_strings flag is currently lost.

and why the _root_ object is affected by this. Because #1113 is just handling the escaping of resolvers, I thought. And this time both resolvers executes...

I haven't looked at it too closely tbh. But I suspect it's related to the fact that #1113 helps avoid issues where the order of interpolation resolution matters when resolving the config. I might be able to provide more details when I look into it more closely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants