Error when creating structured config with untyped Tuple

[cjsg]

Describe the bug OmegConf.create raises the following omegaconf.errors.ConfigIndexError: tuple index out of range when applied to a structured config to a field with type Tuple (i.e., an "untyped" Tuple).

To Reproduce

from dataclasses import dataclass
from typing import Tuple

from omegaconf import OmegaConf


@dataclass
class TupleConfig:
    a_tuple: Tuple[int, int] = (1, 2)


@dataclass
class UntypedTupleConfig:
    a_tuple: Tuple = (1, 2)


# Works as expected.
tuple_config = OmegaConf.create(TupleConfig)
print(tuple_config.__dict__)

# Raises omegaconf.errors.ConfigIndexError: tuple index out of range
untyped_tuple_config = OmegaConf.create(UntypedTupleConfig)

Output

{'a_tuple': [1, 2]}
Traceback (most recent call last):
  File "hydra_bugs/tuple_issue.py", line 22, in <module>
    untyped_tuple_config = OmegaConf.create(UntypedTupleConfig)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/omegaconf.py", line 175, in create
    flags=flags,
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/omegaconf.py", line 861, in _create_impl
    format_and_raise(node=None, key=None, value=None, msg=str(e), cause=e)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/_utils.py", line 741, in format_and_raise
    _raise(ex, cause)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/_utils.py", line 719, in _raise
    raise ex.with_traceback(sys.exc_info()[2])  # set end OC_CAUSE=1 for full backtrace
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/omegaconf.py", line 838, in _create_impl
    flags=flags,
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/dictconfig.py", line 113, in __init__
    format_and_raise(node=None, key=key, value=None, cause=ex, msg=str(ex))
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/_utils.py", line 821, in format_and_raise
    _raise(ex, cause)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/_utils.py", line 719, in _raise
    raise ex.with_traceback(sys.exc_info()[2])  # set end OC_CAUSE=1 for full backtrace
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/dictconfig.py", line 97, in __init__
    self._set_value(content, flags=flags)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/dictconfig.py", line 638, in _set_value
    raise e
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/dictconfig.py", line 635, in _set_value
    self._set_value_impl(value, flags)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/dictconfig.py", line 665, in _set_value_impl
    data = get_structured_config_data(value, allow_objects=ao)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/_utils.py", line 445, in get_structured_config_data
    return get_dataclass_data(obj, allow_objects=allow_objects)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/_utils.py", line 370, in get_dataclass_data
    parent=dummy_parent,
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/omegaconf.py", line 1047, in _maybe_wrap
    ref_type=ref_type,
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/omegaconf.py", line 981, in _node_wrap
    element_type = get_list_element_type(type_)
  File "/home/ec2-user/anaconda3/envs/mscale/lib/python3.7/site-packages/omegaconf/_utils.py", line 618, in get_list_element_type
    if ref_type is not List and args is not None and args[0]:
omegaconf.errors.ConfigIndexError: tuple index out of range
    full_key:
    object_type=None

Expected behavior One would expect omegaconf to handle the UntypedTupleConfig's field a_tuple like a Tuple[Any, ...]. Note that the error comes from the function omegaconf._utils.get_list_element_type function. I think that this function should return the element_type Any.

Additional context

• [ ] OmegaConf version: 2.1.1
• [ ] Python version: 3.7.12
• [ ] Operating system: macOS 12.2.1

[cjsg]

The error arises as follows. At some point, omegaconf calls the function omegaconf._utils.get_list_element_type to determine the type inside of Tuple. This function looks as follows:

def get_list_element_type(ref_type: Optional[Type[Any]]) -> Any:
    args = getattr(ref_type, "__args__", None)
    if ref_type is not List and args is not None and args[0]:
        element_type = args[0]
    else:
        element_type = Any
    return element_type

It gets called with ref_type = typing.Tuple. The problem is that getattr(ref_type, "__args__", None) returns () (and not None). Hence the second line if ref_type is not List and args is not None and args[0]: fails because of args[0], which is an out of range access.

Instead, my guess is that the author of this function expected getattr(ref_type, "__args__", None) to return None in this case, in which case there wouldn t have been any problems, and the output of the function would be Any, as expected.

[Jasha10]

At this time Tuple annotations are not well supported by OmegaConf.

• Length of the tuple is not checked
• Only the first argument to Tuple[...] is registered by OmegaConf, i.e. Tuple[int, float] is treated as equivalent to List[int].

Issue #392 tracks improvement to Tuple support.

[cjsg]

Yes thanks. However, most of the improvements targeted by issue #392 go much further than just correcting what looks like a small bug in get_list_element_type. One would only need to adjust the first or second line there. Other than that, the function seems to be designed to handle a conversion from Tuple to Tuple[Any]. So I think that it could be done independently of the bigger refactor that issue #392 will need.

[Jasha10]

@cjsg Would you be interested in opening a PR to make such a change to get_list_element_type?

[Jasha10]

Other than that, the function seems to be designed to handle a conversion from Tuple to Tuple[Any].

The function was primarily designed to parse the arguments to a List[...] annotation.

>>> get_list_element_type(List[int])
<class 'int'>
>>> get_list_element_type(List[Any])
typing.Any
>>> get_list_element_type(List)
typing.Any

This is the "poor man's" version of the new typing.get_args function, which is not available in Python3.6 (which OmegaConf still partially supports).

Support for Tuple was added later.

[cjsg]

Would you be interested in opening a PR to make such a change to get_list_element_type?

Yes, I can give it a try this week or next one.

[cjsg]

I had a quick look. In get_list_element_type, changing

def get_list_element_type(ref_type: Optional[Type[Any]]) -> Any:
    args = getattr(ref_type, "__args__", None)
    if ref_type is not List and args is not None and args[0]:
        element_type = args[0]
    else:
        element_type = Any
    return element_type

to

def get_list_element_type(ref_type: Optional[Type[Any]]) -> Any:
     args = getattr(ref_type, "__args__", tuple())                       # CHANGED HERE
     if ref_type is not List and args != tuple() and args[0]:        # CHANGED HERE
        element_type = args[0]
    else:
        element_type = Any
    return element_type

works, because getattr(typing.Tuple, "__args__") returns tuple() too. Will do a proper PR later. However, before that, what I do not understand is why on earth we have the if ref_type is not List condition in the original function. For example, why would you want to return Any as element_type if ref_type=List[float]? Wouldn't you want to return float, i.e. args[0]? I somehow have the feeling that this might be a bug too. Can you confirm this?

2 more comments:

• even though just changing the 2 above lines seems to fix the bug, I think that it would be cleaner to also add an if is_tuple_annotation(type_): condition in omegaconf._utils._resolve_forward below the if is_list_annotationcondition. Otherwise, we would get a mismatch at the node creation (in omegaconf._node_wrap, the line starting with node=ListConfig(...)) between element_type, which would beAny, and ref_type, which would beTuple instead of getting case to Tuple[Any] (which would be consistent with what happens when you replace Tuple by List) . Let me know if you have any comments on that.
• I also noticed that the omegaconf._utils.is_generic_list function, reproduced below for convenience, doesn't do what its doc says (neither before nor after my changes to get_list_element_type): typing.List returns True (the doc says False). I think that if we follow the logic of its output condition, then get_list_element_type(ref_type)should return None when ref_type is list, typing.List or typing.Tuple. This is currently not the case (returns typing.Any in all 3 cases), and returning None would actually break many other things. So probably one should probably re-think the logic of is_generic_list function. (But probably not high priority, since this function seems to play little role in the code elsewhere).

[Jasha10]

Apologies for the delayed reply.

However, before that, what I do not understand is why on earth we have the if ref_type is not List condition in the original function.

It is because List[float] is not List :)

>>> ref_type = List[float]
>>> ref_type is not List
True

We return Any if ref_type == List, but not if ref_type == List[float].

[Jasha10]

I also noticed that the omegaconf._utils.is_generic_list function, ... doesn't do what its doc says ... (But probably not high priority, since this function seems to play little role in the code elsewhere).

Hmm 🤔 I agree with your comments about is_generic_list.

[Jasha10]

it would be cleaner to also add an if is_tuple_annotation(type_): condition in omegaconf._utils._resolve_forward below the if is_list_annotationcondition. Otherwise, we would get a mismatch at the node creation ...

That sounds good to me!