Deserialize using the default values if it exists

[Edresson]

No description provided.

[Edresson]

I'm not sure if it was a spelling mistake but if it wasn't I added the check for both MISSING object (_MISSING) and MISSING string(MISSING)

[Edresson]

This check does not get the complex type (like List[int] and others). The default values of complex type fields will be ignored. To deal with that, I added here a check if the default value is not "MISSING" we use it instead of raising the error. So if we need that the user provides some param it needs to be defined as "MISSING" type (string or object).

[reuben]

Can you add a test that fails before this PR is applied but passed with it applied?

[Edresson]

Can you add a test that fails before this PR is applied but passed with it applied?

Without this PR will break all times that we define a complex type (like List[int]) and do not provide it as data in .new_from_dict() or from_dict(). The default value will be ignored and it will raise an error.
I think that defaults values need to be used, given that it is not a Required field. I add a new field in the test_init_from_dict unit test to show the "error". I'm not so familiar with Coqpit yet. If you know a better way to solve this, please let me know :).

[reuben]

So while reviewing I noticed that you changed both deserialize and deserialize_immutable, but the test only calls init_from_dict, which uses deserialize_immutable. So I added the following test on the main branch, to make sure deserialize is being handled properly:

diff --git a/tests/test_init_from_dict.py b/tests/test_init_from_dict.py
index 2e14d3f..2a4dc46 100644
--- a/tests/test_init_from_dict.py
+++ b/tests/test_init_from_dict.py
@@ -21,6 +21,8 @@ class Reference(Coqpit):
             Person(name="Ceren", age=15),
         ]
     )
+    people_ids: List[int] = field(
+        default_factory=lambda: [1, 2, 3])
 
 
 @dataclass
@@ -31,6 +33,9 @@ class WithRequired(Coqpit):
 def test_new_from_dict():
     ref_config = Reference(name="Fancy", size=3 ** 10, people=[Person(name="Anonymous", age=42)])
 
+    in_place_deserialize = Reference()
+    in_place_deserialize.from_dict({"name": "Fancy", "size": 3 ** 10, "people": [{"name": "Anonymous", "age": 42}]})
+
     new_config = Reference.new_from_dict(
         {"name": "Fancy", "size": 3 ** 10, "people": [{"name": "Anonymous", "age": 42}]}
     )

But it passes even before this change. Do you have a good idea of what's going on? Should we keep the changes only to deserialize_immutable or is there something deeper that needs to be figured out here?

[Edresson]

So while reviewing I noticed that you changed both deserialize and deserialize_immutable, but the test only calls init_from_dict, which uses deserialize_immutable. So I added the following test on the main branch, to make sure deserialize is being handled properly:

diff --git a/tests/test_init_from_dict.py b/tests/test_init_from_dict.py
index 2e14d3f..2a4dc46 100644
--- a/tests/test_init_from_dict.py
+++ b/tests/test_init_from_dict.py
@@ -21,6 +21,8 @@ class Reference(Coqpit):
             Person(name="Ceren", age=15),
         ]
     )
+    people_ids: List[int] = field(
+        default_factory=lambda: [1, 2, 3])
 
 
 @dataclass
@@ -31,6 +33,9 @@ class WithRequired(Coqpit):
 def test_new_from_dict():
     ref_config = Reference(name="Fancy", size=3 ** 10, people=[Person(name="Anonymous", age=42)])
 
+    in_place_deserialize = Reference()
+    in_place_deserialize.from_dict({"name": "Fancy", "size": 3 ** 10, "people": [{"name": "Anonymous", "age": 42}]})
+
     new_config = Reference.new_from_dict(
         {"name": "Fancy", "size": 3 ** 10, "people": [{"name": "Anonymous", "age": 42}]}
     )

But it passes even before this change. Do you have a good idea of what's going on? Should we keep the changes only to deserialize_immutable or is there something deeper that needs to be figured out here?

Yeah, on my side it only broke in deserialize_immutable. I added as well in deserialize because I thought probably it will broken without. I don't know if it is good to keep or not.

[reuben]

Then let's take it out since we don't understand it, better to add if if a real need is identified and understood. Other than that LGTM! Thanks for the PR

[Edresson]

Then let's take it out since we don't understand it, better to add if if a real need is identified and understood. Other than that LGTM! Thanks for the PR

Done :)