konradhalas / dacite

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

Dacite doesn't play well with freezegun #185

Closed ehiggs closed 1 year ago

ehiggs commented 2 years ago

freezegun is a super common library used for testing and basically indispensable. But it seems when working with dacite, it can run into issues. As follows:

>>> from freezegun import freeze_time
>>> from dateutil.parser import isoparse
>>> from dacite import from_dict, Config
>>> from dataclasses import dataclass
>>> @dataclass
... class Foo:
...     timestamp: datetime
... 

# -- parsing Foo from_dict should work like this, and we see it works fine.
>>> from_dict(Foo, {"timestamp": "2022-04-04T15:37:51.258Z"}, config=Config({datetime: isoparse}))
Foo(timestamp=datetime.datetime(2022, 4, 4, 15, 37, 51, 258000, tzinfo=tzutc()))

# -- but if time is frozen and isoparse returns a FakeDatetime then from_dict blows up.
>>> with freeze_time("2020-06-23T12:15:00.256Z"):
...     from_dict(Foo, {"timestamp": "2022-04-04T15:37:51.258Z"}, config=Config({datetime: isoparse}))
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/Users/ehiggs/src/dacite-test/venv/lib/python3.8/site-packages/dacite/core.py", line 68, in from_dict
    raise WrongTypeError(field_path=field.name, field_type=field.type, value=value)
dacite.exceptions.WrongTypeError: wrong value type for field "timestamp" - should be "datetime" instead of value "2022-04-04T15:37:51.258Z" of type "str"

dacite-1.6.0-py3-none-any.whl

konradhalas commented 1 year ago

@ehiggs thanks for reporting this issue.

As you probably know freezegun monky patches datetime within with block, so yours config=Config({datetime: isoparse}) is config=Config({freezegun.api.FakeDatetime: isoparse}) but Foo.timestamp is still datatime from the Python standard library. Those types do not match so dacite doesn't use this type hook at all.

This will work (both datetimes are monky patched):

with freeze_time("2020-06-23T12:15:00.256Z"):
    @dataclass
    class Foo:
        timestamp: datetime

    from_dict(Foo, {"timestamp": "2022-04-04T15:37:51.258Z"}, config=Config({datetime: isoparse}))

TBH I don't see any clean solution - I don't want to add freezgun as a dependency to my project.

Freezgun is really awesome library (I use it everyday) but because of this hacky monky patching it sometimes doesn't cooperate seamlessly with other tools.

I'm closing this issue for now, but feel free to comment/reopen/provide PR ;)

ehiggs commented 1 year ago

Thanks for the reply. The proposed solution is less than ideal because it means all imports for tests need to take place in a freeze_time context.

Digging into the code a bit more, the issue seems to be when we check if the type parsed correctly:

            if config.check_types and not is_instance(value, field.type):
                raise WrongTypeError(field_path=field.name, field_type=field.type, value=value)

So this should fire if the parsed type is not datetime.

Let's make a dt of type FakeDatetime:

>>> with freeze_time("2022-01-01"):
...     dt = datetime.now()
... 
>>> type(dt)
<class 'freezegun.api.FakeDatetime'>
>>> from freezegun.api import FakeDatetime
>>> isinstance(dt, FakeDatetime)
True

Great. Note, this is also isinstance of datetime:

>>> isinstance(dt, datetime)
True

But dacite uses a special is_instance function:

>>> from dacite.types import is_instance
>>> is_instance(dt, datetime)
True
>>> is_instance(dt, FakeDatetime)
True

So a FakeDatetime IS a datetime. So why would this exception be raisd? 🤔

SnijderC commented 1 year ago

Problem is not that the type doesn't match, problem is that before that, the type hook was not triggered, leading the expected datetime here to remain a str type as you can also see in the first post's error message.

field "timestamp" - should be "datetime" instead of value "2022-04-04T15:37:51.258Z" of type "str"

That is caused by the fact that the list of type_hooks we pass is defined like this: Config(type_hooks={datetime: isoparse}) [added the keyword arg for visibility). When run from a test with freezetime, the type datetime monkey patched to be freezegun's FakeDatetime. The call to transform_value within from_dict will now not match datetime with FakeDatetime, not do the expected transformation, thus returning the original string, which is obviously of type str. This triggers the WrongTypeError exception.

Screenshot 2023-02-16 at 18 04 16

So this:

So a FakeDatetime IS a datetime. So why would this exception be raisd? 🤔

is because the type here is in fact not FakeDatetime or datetime, it is str.

Doesn't solve them problem, but knowing this we can discuss whether the type_hooks behaviour can be modified, as opposed to the type comparison.

ErikBrendel commented 1 year ago

A possible workaround to have freezegun and dacity both running:

# define a helper dict wrapper
class FreezegunCompatibleTypeHookWrapper(dict):
    def __contains__(self, item):
        return super().__contains__(item) or str(item) == "<class 'datetime.datetime'>"

    def __getitem__(self, item):
        if str(item) == "<class 'datetime.datetime'>":
            # replace the datetime class with the (maybe monkeypatched) directly imported datetime module
            # reason: https://github.com/konradhalas/dacite/issues/185#issuecomment-1433506657
            item = datetime
        return super().__getitem__(item)

# and then use:
Config(type_hooks=FreezegunCompatibleTypeHookWrapper({datetime: isoparse}))