DictConfig silently ignores setdefault

[an1lam]

If I run the following code using omegaconf version 2.0.0 (Python 3.7.6, anaconda version):

import omegaconf

if __name__ == "__main__":
    cfg = omegaconf.DictConfig({"a": "b"})
    print(cfg)

    cfg.setdefault("c", [])
    print(cfg)

    cfg["c"] = []
    print(cfg)

I'll get the following output:

2.0.0
{'a': 'b'}
{'a': 'b'}
{'a': 'b', 'c': []}

Observe that 'c' does not get set by setdefault even though DictConfig inherits from MutableMapping, which in its spec, states that it should support setdefault.

I feel like this should in order of preference:

• work;
• throw an exception rather than silently fail;
• be documented in the DictConfig API.

Since I'm assuming none of these are that hard to implement, I could potentially implement the preferred solution myself. Just would want some guidance on which you think makes the most sense and, if it's the first option, what's the right way to do it.

[omry]

Hi, it's really curious that this went undetected because OmegaConf is passing a strict mypy check.

Since the intention is for OmegaConf DictConfig to be as compatible as possible with dict (with the exception of the base class) - I would like this to work.
go ahead and send a PR adding it, be sure to add tests covering it and a news fragment (this is more or less correct for OmegaConf too).

[an1lam]

So it's because the method exists on the underlying base class, but it relies on self[key] throwing KeyError when it fails. One way to fix this would be to override the method, another would be to throw KeyError when bracket accesses fail. Which do you prefer?

[omry]

[] throws a key error when struct mode is enabled. this is by design. changing this behavior for regular mode is a significant breaking change.

Let's have our own implementation
be sure to test it with and without struct mode.

[omry]

If you are using OmegaConf through Hydra, struct mode is becoming the default in 1.0.
If you are using it directly, you can just set the flag after initialization.

One more thing:
Use OmegaConf.create() to create the DictConfig object, users are not expected to create it directly and I make no promises with respect to breaking API changes for it's constructor.

[omry]

Since you are actively working on it, setting the milestone to 2.0.1.

[an1lam]

👍 just to set expectations, I probably won't get to it until the weekend.

[omry]

That's fine.