konradhalas / dacite

Simple creation of data classes from dictionaries.
MIT License
1.72k stars 106 forks source link

Regression in handling type_hooks for dict keys between dacite version 1.7 and 1.8 #217

Open kkg4theweb opened 1 year ago

kkg4theweb commented 1 year ago

Describe the bug

Type hooks don't seem to be respected for dict keys. I have been using type-hooks to map string/name of enum values of a dict to the corresponding python enum. This stopped working in dacite version 1.8 but is fine with version 1.7

To Reproduce Here is the code snippet:

import dataclasses
import dacite
import enum
import typing

@enum.unique
class FooEnum(enum.IntEnum):
  FOO = 1
  BAR = 2

type_hooks = {}
type_hooks[FooEnum] = lambda raw_str: FooEnum[raw_str]

dacite_cfg = dacite.Config(strict_unions_match=True, strict=True, type_hooks=type_hooks)

@dataclasses.dataclass(frozen=True)
class EnumKeyMapTypes:
  string_int_dict_val: typing.Dict[FooEnum, int]

dc_val = EnumKeyMapTypes(string_int_dict_val={
            FooEnum.FOO: 1,
            FooEnum.BAR: 2
        })

py_dict_val = {
        'string_int_dict_val': {
            'FOO': 1,
            'BAR': 2
        }
}

dacite.from_dict(EnumKeyMapTypes, py_dict_val, config=dacite_cfg)

Expected behavior

The dacite.from_dict should result in the following:

EnumKeyMapTypes(string_int_dict_val={<FooEnum.FOO: 1>: 1, <FooEnum.BAR: 2>: 2})

Environment

Additional context

This works as expected for dacite version 1.7 but is broken in version 1.8 with the following error:

WrongTypeError                            Traceback (most recent call last)
[<ipython-input-8-4d0ead28977b>](https://localhost:8080/#) in <module>
----> 1 dacite.from_dict(EnumKeyMapTypes, py_dict_val, config=dacite_cfg)

[/usr/local/lib/python3.8/dist-packages/dacite/core.py](https://localhost:8080/#) in from_dict(data_class, data, config)
     67                 raise
     68             if config.check_types and not is_instance(value, field_type):
---> 69                 raise WrongTypeError(field_path=field.name, field_type=field_type, value=value)
     70         else:
     71             try:

WrongTypeError: wrong value type for field "string_int_dict_val" - should be "typing.Dict[__main__.FooEnum, int]" instead of value "{'FOO': 1, 'BAR': 2}" of type "dict"

Google colab broken (1.8): https://colab.research.google.com/drive/1BBMdjkmZow12V_1UlX1dB_jJWWjBx1cv?usp=sharing Google colab working (1.7): https://colab.research.google.com/drive/1z1-qn5ER-FHj7VdOKKlsv7ZzjXm5wekY?usp=sharing

sunmy2019 commented 1 year ago

After some bug fixes since 1.7.0, now these lines was responsible for converting dicts.

https://github.com/konradhalas/dacite/blob/02ee99348d4c8354fa309b8d1f3525dafda592e6/dacite/core.py#L140-L142

Clearly, we see that from line 142, key is not applied with type_hooks in the config.

https://github.com/konradhalas/dacite/blob/02ee99348d4c8354fa309b8d1f3525dafda592e6/dacite/core.py#L142

Don't know if the author was intended, but a quick fix would be direct, as chaning line 141-142 to

        types = extract_generic(collection, defaults=(Any, Any))
        return data_type(
            (
                _build_value(type_=types[0], data=key, config=config),
                _build_value(type_=types[1], data=value, config=config),
            )
            for key, value in data.items()
        )
joachim-j commented 1 year ago

Below is probably a duplicate from the above issue, just wanted to make sure. Specifically, using cast=[...] to configure your dacite config instead of the type_hooks parameter also causes an issue on 1.8.0 (I have knowledge of it working on 1.6.0, didn't test 1.7.0)

from enum import Enum
from typing import Dict

from dacite import from_dict, Config

class Foo(Enum):
    EVALUE = 'enum_value'

@dataclass
class FooConfig:
    foo: Dict[Foo, str]

data = {"foo": {"enum_value": "bar"}}
# next line throws error in 1.8.0, doesn't in 1.6.0
config = from_dict(data_class=FooConfig, data=data, config=Config(cast=[Foo]))
mciszczon commented 1 year ago

After some bug fixes since 1.7.0, now these lines was responsible for converting dicts.

https://github.com/konradhalas/dacite/blob/02ee99348d4c8354fa309b8d1f3525dafda592e6/dacite/core.py#L140-L142

Clearly, we see that from line 142, key is not applied with type_hooks in the config.

https://github.com/konradhalas/dacite/blob/02ee99348d4c8354fa309b8d1f3525dafda592e6/dacite/core.py#L142

Don't know if the author was intended, but a quick fix would be direct, as chaning line 141-142 to

        types = extract_generic(collection, defaults=(Any, Any))
        return data_type(
            (
                _build_value(type_=types[0], data=key, config=config),
                _build_value(type_=types[1], data=value, config=config),
            )
            for key, value in data.items()
        )

Thanks for the details! The issue seems to stem from something else, though, as checking as back as v1.6.0, the key was never being passed through _build_value. We'll investigate it some more!

object-Object commented 1 year ago

Thanks for the details! The issue seems to stem from something else, though, as checking as back as v1.6.0, the key was never being passed through _build_value. We'll investigate it some more!

@mciszczon Here's some investigation for you. In 1.7.0, the code path was as follows:

So the bug in _build_value_for_collection(), where the key wasn't being transformed, never actually manifested because the same thing was being done (without the bug) earlier in the process.

In 1.8.1, transform_value() was removed and its functionality was moved to _build_value(). The key transformation was overlooked, and this bug was introduced.