Interpolations can't refer to non-string keys

[odelalleau]

Describe the bug

If a DictConfig is using non-string keys, interpolations can't access them:

cfg = OmegaConf.create({0: 0, 1: "${0}"})
cfg[1]

InterpolationKeyError: Interpolation key '0' not found
    full_key: 1
    object_type=dict

There are probably similar issues with other non-string key types.

Expected behavior

Ideally, I think keys should be cast to their corresponding type according to the grammar (which would need to be updated to allow this). Previous behavior could be preserved to maintain backward compatibility and keep things simple and intuitive (i.e., ${0} could still refer to the node with key "0" if there is no node with key 0)

Side note

OmegaConf.select() doesn't seem to support non-string keys either.

[omry]

Non string keys are new in 2.1, so there are no backward compatibility concerns. I am okay with not supporting them in interpolations for now, but it's better to raise an exception when an attempt to interpolate into a dictionary with non string keys is detected.

I am also okay with adding support for it, but I think it's optional for 2.1 (not instead of anything already in the 2.1 list).

[odelalleau]

Non string keys are new in 2.1, so there are no backward compatibility concerns.

The potential concern would be that if we implement it, then ${0} may not work anymore on a DictConfig with a string key "0" (while it works now).

I am okay with not supporting them in interpolations for now, but it's better to raise an exception when an attempt to interpolate into a dictionary with non string keys is detected.

Since interpolations are based on _select_impl(), that's probably the best place to add such a check (so that OmegaConf.select() also raises the same exception). Not sure it's worth it though because most DictConfigs have a key_type set to Any (even structured configs). I guess we could try and keep track of the types of keys that have been added so far, but it seems too cumbersome just for this purpose.

I am also okay with adding support for it, but I think it's optional for 2.1 (not instead of anything already in the 2.1 list).

Agreed, I'd rather not try and address it in 2.1.

[omry]

Non string keys are new in 2.1, so there are no backward compatibility concerns.

The potential concern would be that if we implement it, then ${0} may not work anymore on a DictConfig with a string key "0" (while it works now).

DictConfig with Any can also have both "0" and 0 as keys at the same time.

In [12]: cfg = OmegaConf.create({1: 1, "1": 2})

In [13]: cfg[1]
Out[13]: 1

In [14]: cfg["1"]
Out[14]: 2

I guess your concern is that implementing your proposal to use the grammar to differentiate will break such cases.

I also have concerns about the ability of a regex to handle all cases:

a.b
a.'b.c'
# not sure we support interpolation in keys, but if we do it can get difficult:
a.'b.${c}'
a.'b.${c.\'d\'}'

I think it makes more sense to me to use the type declared on the DictConfig. If it's Any, we can assume it's a string. If it's an int, we convert string to an int etc. In the case of Any, we may want to reject inserting a key if an existing key maps to the same string (In fact, we may add this restriction for 2.1).

[odelalleau]

I guess your concern is that implementing your proposal to use the grammar to differentiate will break such cases.

My concern was simply to ensure that {"0": 1, "x": "${0}"} still works.

I also have concerns about the ability of a regex to handle all cases:

a.b
a.'b.c'
# not sure we support interpolation in keys, but if we do it can get difficult:
a.'b.${c}'
a.'b.${c.\'d\'}'

Right now we don't support interpolations in node keys (this is #610). I think the kind of stuff you described could be made to work if we want to (but that would be also relying on the ANTLR parser, not just a regex).

I think it makes more sense to me to use the type declared on the DictConfig. If it's Any, we can assume it's a string. If it's an int, we convert string to an int etc. In the case of Any, we may want to reject inserting a key if an existing key maps to the same string (In fact, we may add this restriction for 2.1).

That would probably work too.

[omry]

Tentative for 2.1, if we can fix it without grammar change.

[omry]

Bouncing to 2.2.