Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow type_hooks for substituted objects of the same parent type #219

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

SnijderC
Copy link

We've had issues with dacite not supporting type_hooks on datetime during testing, when freezegun is monkey patching the datetime object to be FakeDatetime, see here.

At first it was assumed that the dataclass attribute was monkey patched and therefore the type_hooks list doesn't have a match for the attribute's type. However this is not the case, as the interpreter would have already interpreted the type annotation before the monkey patch takes place, taking a reference to the original object.
It is instead the type_hooks' key (which for clarity is not a string but instead the data type itself) that is patched. Thus on structuring data into a dataclass, the type_hooks dict contains a FakeDatetime key to an isoparse hook.

Last bit of context needed before going to the proposed fix: the way type hooks are matched with dataclass's attributes's types in dacite today is by in, which is like iterating over the keys and doing a value comparison (==, not is). This does however not allow us to have objects to be replaced following the Liskov Principle, whereby we can substitute an object of one type by another so long as they share the same parent (simplified for brevity, sufficient for this context).

Coming to the proposal: If we change the way we compare type_hooks with "is a ..." logic instead of "==" logic, we can substitute objects that should provide the same behaviour.

Getting back to the original problem, freezegun. Since Python allows us to redefine the behaviour of objects via meta classes, we can create objects that aren't actual drop-ins but will still declare themselves as a subclass of another type. This is exactly what freezegun is doing here. This object is not a subclass of datetime.datetime by inheritance, instead by explicitly overriding the comparison made by issubclass. With the proposed change, this means we can monkey patch datetime.datetime to FakeDateTime and still have a matching type hook in the type_hooks dict. I imagine this will be helpful for a lot more cases where monkey patching is happening with objects defined in a similar way.

@SnijderC
Copy link
Author

SnijderC commented Apr 6, 2023

@konradhalas I appreciate you probably develop this in your free time, don't want to bother you, but any chance you could review this please? :)

@mciszczon
Copy link
Collaborator

@SnijderC Hi there! We will look into open PRs and issues soon, sorry for the delay!

@psi29a
Copy link

psi29a commented Jun 29, 2023

To quote a song: How soon is now? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants