structure raises a KeyError when optional attribute is missing

[cmaddalozzo]

• cattrs version: 22.1.0
• Python version: 3.9.10
• Operating System: macOS Big Sur (11.5.2)

Description

Passing a dictionary to structure that has missing entries for Optional values results in an error. Instead I would expect the instance to get built successfully with the optional property unset.

What I Did

Example code:

from cattrs import structure
from attrs import define
from typing import Optional


@define
class Dog:
    name: str
    nickname: Optional[str]


if __name__ == "__main__":
    # This works
    spike = structure({"name": "Spike", "nickname": "Carl"}, Dog)
    # This raises an error
    rex = structure({"name": "Rex"}, Dog)

The result:

  + Exception Group Traceback (most recent call last):
  |   File "/Users/cmadd/tractable/graphs/cattrs_bug.py", line 13, in <module>
  |     rex = structure({"name": "Rex"}, Dog)
  |   File "/Users/cmadd/tractable/graphs/venv/lib/python3.9/site-packages/cattrs/converters.py", line 281, in structure
  |     return self._structure_func.dispatch(cl)(obj, cl)
  |   File "<cattrs generated structure __main__.Dog>", line 14, in structure_Dog
  |     if errors: raise __c_cve('While structuring Dog', errors, __cl)
  | cattrs.errors.ClassValidationError: While structuring Dog
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "<cattrs generated structure __main__.Dog>", line 10, in structure_Dog
    |     res['nickname'] = __c_structure_nickname(o['nickname'], __c_type_nickname)
    | KeyError: 'nickname'
    | Structuring class Dog @ attribute nickname
    +------------------------------------

[Tinche]

Hello! You'll need to add a default value yourself to get this behavior. So just change your class to be

@define
class Dog:
    name: str
    nickname: Optional[str] = None

cattrs generally avoids making these calls for you. Maybe you'd like the field to default to an empty string instead? So you're expected to provide the default value.

[cmaddalozzo]

@Tinche ahh, thanks so much for the quick reply! I thought I must have been missing something simple 🤦 .

Should it maybe be added to the example in the README?

@define
class Dog:
    cuteness: int
    chip: Optional[DogMicrochip]

[Tinche]

Sure! Want to send a quick PR? :)

[kulych]

Hi!

I encountered the same problem, but using default = None is not a possible solution for me, as I would have to reorder the fields in my class so that those with defaults are the last ones.
Even if those fields were already suitably ordered, the same problem would occur if I had to add a new non-default field in the future.

Each reordering is a breaking change which is a no-go in my case.

This also can't be solved by a custom MyOptional type, because the attribute access o['myField'] is already performed on "one level before".

I managed to solve it by modifying make_dict_structure_func by doing o.get('myField') instead of direct [] access.

from typing_inspect import is_optional_type

def make_dict_structure_fn_optional(...):
    ...
                if is_optional_type(a.type):
                    lines.append(
                        f"{i}res['{ian}'] = {struct_handler_name}(o.get('{kn}'), {type_name})"
                    )
                else:
                    lines.append(
                        f"{i}res['{ian}'] = {struct_handler_name}(o['{kn}'], {type_name})"
                    )
    ...

This works for me, because I always want to set missing Optional fields to None, but I am not sure if this is a generally acceptable solution.

What is your opinion? How would you approach the "can't use = None default" situation?

BTW: Thanks for this great library :-)

[Tinche]

Hello!

using default = None is not a possible solution for me, as I would have to reorder the fields in my class so that those with defaults are the last ones.

But attrs classes and dataclasses must have fields with defaults after the fields with no defaults anyway, right?

Thanks for this great library :-)

You're welcome, thanks for using ;)

[kulych]

But attrs classes and dataclasses must have fields with defaults after the fields with no defaults anyway, right?

My class has no defaults and looks like this:

@frozen
class MyClass:
    a_float: float
    maybe_int: Optional[int]
    a_string: str

I might receive data (without maybe_int) {"a_float": 0.1, "a_string": "hello"}, which should be structured to MyClass(0.1, None, "hello"). This however does not work and I get a KeyError while trying to call .structure on such data.

If I wanted to use = None as suggested, I would have to reorder fields such that

@frozen
class MyClass:
    a_float: float
    a_string: str
    maybe_int: Optional[int] = None

which breaks MyClass(1.5, None, "hello") calls in some third-party code. So I prefer to keep the field order as is and to not specify = None at all.

[Tinche]

Ah I see, alright. And you'd like to do this just for Optionals?

Gonna think about this a little.

[kulych]

Ah I see, alright. And you'd like to do this just for Optionals?

Maybe as well for Union[T1, T2, ..., None], but that should be the same thing. The adjustment I provided uses a function is_optional_type which works for those as well.

Here is the behavior I am currently able to get by using the adjustment provided above.

from typing import Any, Optional, Union

import pytest
from attr import frozen, has
from cattr import GenConverter
from cattrs.errors import ClassValidationError

CONVERTER = GenConverter()


def custom_optional_hook_factory(cls):
    # Custom make_dict_structure_fn which calls .get() instead of [] when needed
    return make_dict_structure_fn_optional(
        cls,
        CONVERTER,
    )


CONVERTER.register_structure_hook_factory(has, custom_optional_hook_factory)


@frozen
class WithOptionals:
    required_field: bool
    un: Union[bool, str]
    un_opt: Union[bool, str, Optional[int]]
    a: Optional[int]
    b: Optional[float] = None
    c: Optional[str] = "custom default"


def test_structure_with_optionals() -> None:
    # Unions must have custom hooks
    CONVERTER.register_structure_hook(
        Union[bool, str], lambda v, _: v if isinstance(v, bool) else str(v)
    )

    def structure_union_bool_str_optint(v: Any, _: Any) -> Union[bool, str, Optional[int]]:
        if isinstance(v, bool):
            return v
        if isinstance(v, int):
            return v
        if v is None:
            return None
        return str(v)

    CONVERTER.register_structure_hook(
        Union[bool, str, Optional[int]], structure_union_bool_str_optint
    )

    # required_field is not present and is not Optional
    with pytest.raises(ClassValidationError):
        CONVERTER.structure({}, WithOptionals)

    # required_field is present, but un field is not present and is not Optional
    with pytest.raises(ClassValidationError):
        CONVERTER.structure({"required_field": True}, WithOptionals)

    assert CONVERTER.structure(
        {"required_field": True, "un": "hey"}, WithOptionals
    ) == WithOptionals(
        required_field=True,
        un="hey",
        un_opt=None,  # not present in dict and is implicitly set to None
        a=None,  # not present in dict and is implicitly set to None
        b=None,  # not present in dict and has default None
        c="custom default",  # not present in dict and has custom default
    )

    assert CONVERTER.structure(
        {"required_field": True, "b": 0.1, "un": False}, WithOptionals
    ) == WithOptionals(
        required_field=True,
        un=False,
        un_opt=None,
        a=None,
        b=0.1,  # present in dict
        c="custom default",
    )

    assert CONVERTER.structure(
        {"required_field": True, "a": 123, "b": 0.1, "un": True}, WithOptionals
    ) == WithOptionals(
        required_field=True,
        un=True,
        un_opt=None,
        a=123,  # present in dict
        b=0.1,  # present in dict
        c="custom default",
    )

    assert CONVERTER.structure(
        {
            "required_field": True,
            "a": 123,
            "b": 0.1,
            "c": "hey",
            "un": "test",
            "un_opt": "i am here",
        },
        WithOptionals,
    ) == WithOptionals(
        required_field=True,
        un="test",
        un_opt="i am here",
        a=123,  # present in dict
        b=0.1,  # present in dict
        c="hey",  # present in dict
    )

Gonna think about this a little.

Thanks :)