Closed rominf closed 5 years ago
I've tested your changes, and it seems to work great.
@harmon, can you provide the tests?) I forgot about them and I don't have time now.
@rominf thank you very much for your PR, I fixed this issue in a very similar way (without unnecessary list comprehension) + tests in this commit 96ee320765af5817be22d213d00aac0813715c6c. Once again thank you :)
So, I started to write tests for this, but realized that if you have something like:
@dataclass
class MyClassA:
a: str
@dataclass
class MyClassB:
propA: Union[str, List, MyClassA]
MyClassB({'propA': {'a': 'fun'}})
That things don't work so well. Just something I couldn't quite get past in the limited time I tried. Basically, I tried to duplicate all tests that tested List, and replace it with Dict to make sure they all still worked, and copying this one and adding MyClassA at the end of the Union seemed to break:
I'm not sure that's a stopper to release this, but something I noticed.
@harmon thank you for reporting, I will check it.
Here's the tests I wrote to try and make sure the different combos didn't break. I think the issue was that it couldn't figure out the difference between a dataclass object or a Dict, since passing a dictionary could be for either. So you'd have to like try to create them each, catch any errors, and then try the next option if it raised an error. Something like that.
diff --git a/tests.py b/tests.py
index 6d7f7c3..e9b0867 100644
--- a/tests.py
+++ b/tests.py
@@ -1,5 +1,5 @@
-from dataclasses import dataclass, field
-from typing import Optional, List, Set, Union, Any
+from dataclasses import dataclass, field, asdict
+from typing import Optional, List, Set, Union, Any, Dict, Mapping
import pytest
@@ -398,6 +398,18 @@ def test_from_dict_with_set_of_dataclasses():
assert result == Y(x_set={X(i=1), X(i=2)})
+def test_from_dict_with_dict_of_dataclasses():
+ @dataclass
+ class X:
+ i: int
+
+ @dataclass
+ class Y:
+ x_dict: Dict[str, X]
+
+ result = from_dict(Y, {'x_dict': {'x_1': {'i': 1}, 'x_2': {'i': 2}}})
+
+ assert result == Y(x_dict={'x_1': X(i=1), 'x_2': X(i=2)})
def test_from_dict_with_optional_list_of_dataclasses():
@dataclass
@@ -412,6 +424,18 @@ def test_from_dict_with_optional_list_of_dataclasses():
assert result == Y(x_list=[X(i=1), X(i=2)])
+def test_from_dict_with_optional_dict_of_dataclasses():
+ @dataclass
+ class X:
+ i: int
+
+ @dataclass
+ class Y:
+ x_dict: Optional[Dict[str, X]]
+
+ result = from_dict(Y, {'x_dict': {'x_1': {'i': 1}, 'x_2': {'i': 2}}})
+
+ assert result == Y(x_dict={'x_1': X(i=1), 'x_2': X(i=2)})
def test_from_dict_with_wrong_filed_name_in_config_remap():
@dataclass
@@ -567,6 +591,22 @@ def test_from_dict_with_union_of_data_classes_collections_with_correct_data():
assert result == Z(x_or_y=[Y(s='test')])
+def test_from_dict_with_union_of_data_classes_collections_with_correct_data():
+ @dataclass
+ class X:
+ i: int
+
+ @dataclass
+ class Y:
+ s: str
+
+ @dataclass
+ class Z:
+ x_or_y: Union[List[X], Dict[str, Y]]
+
+ result = from_dict(Z, {'x_or_y': {'y_1': {'s': 'test'}}})
+ assert result == Z(x_or_y={'y_1': Y(s='test')})
@harmon can you test latest version (with 3f818c7538b116954d86f89bb3e3673942220f3f, not on PyPI yet)?
Hi! Ok, those tests pass now! I had a few others too I forgot to post, and they all work. I think it's all good now. Thanks!
I'm gonna recommend to my coworkers that this is the way to go. This library is more complex than what we were using before (different process entirely, though), but it's using standard objects under the hood, so I really like it. And it's frickin' unit tested! We had a previous thing that was a custom implementation of Dict to do what I want to use DataClasses for now, and while less lines of code, it was horrible to reason about or manipulate.
Python doesn't support this deeply nested unpacking into Dataclasses, which is something we'd really like to do, so I'm glad I stumbled across this library.
Thanks for your work!
@harmon great! I've just released v0.0.20 on PyPI. Thank you for sharing my project with your team :)
Oh! Can this be merged? I'd love this feature. It's a blocker for me adopting this library, which I'd really like to use.