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

Fixing deserialization issues for dataclasses with subgroup fields and nested dataclasses #211

Closed
wants to merge 11 commits into from

Conversation

zhiruiluo
Copy link
Contributor

@zhiruiluo zhiruiluo commented Jan 24, 2023

This PR is the implementation for the contribution 1 mentioned in #197.

  • The default values of using add_selected_subgroups and parse_selection were set to False.
  • Tests were added to verify the backward-compatiability of to_dict and from_dict.
  • Added fix for issue The deserialization failed on nested dataclasses. #210
    • check whether default_factory is dataclass
    • check whether the function of partial of default_factory is dataclass
    • recursively call from_dict when either condition is true
  • Questions still open to answer
    • whether check field.default for nested dataclass.

zhiruiluo added a commit to zhiruiluo/SimpleParsing that referenced this pull request Jan 31, 2023
lebrice added a commit that referenced this pull request Feb 3, 2023
* add subgroups info on serialization

* fix issue #210 fail on nested dataclasses

* add tests

* apply pre-commit hooks

* remove log

* renaming, rearranging, and adding comments for readability

* init commit

* rebase and add tests

* check whethe the input of  is dataclass instead of dictionary

* init commit

* rebase and add tests

* make sure entries of new_values of replace function are unflattened

* fix replace docstring

* verify the type of raw_value when it is dataclass for subgroups, and disable it for nested dataclass

* Set add_selected_subgroups to false and remove nested subgroups tests

* fix test_replace.py

* checkout serializable from deserialization_fix

* replace raise TypeError for non dataclass instance

* add test for mixed flatten and nested dictionary

* remove changes from PR #211 and skip tests related to issue nested dataclass #210

* Update simple_parsing/replace.py

utilze dataclasses.replace to simplify simple_parsing.replace.

Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>

* revise replace with dataclasses.replace and add test_replace_union_dataclass

* Small formatting change in `replace.py`

---------

Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
@lebrice
Copy link
Owner

lebrice commented Mar 13, 2023

Closing in favor of #233

@lebrice lebrice closed this Mar 13, 2023
@zhiruiluo zhiruiluo deleted the deserialization_fix branch April 28, 2023 04:47
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.

2 participants