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.

Sign up for GitHub

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

Jump to bottom

improve Union deserialization when "__type" field specifier is not present #478

Merged
merged 6 commits into from
Sep 12, 2023
Merged

improve Union deserialization when "__type" field specifier is not present #478

merged 6 commits into from
Sep 12, 2023

Conversation

idbentley
Copy link
Contributor

This pull request adds a best-effort deserialization to Union types when no __type field specifier is present.

In the case that no __type field is present, a warning is issued describing the risk of such unmarshalling attempt.

We attempt to decode ensuring that keys exist on the type, and the values are appropriate. If either of these conditions fails, we abandon that dataclass type, and move onto the next one.

If no type is found, a warning is issued, the value is not decoded, and is left as-is.

@idbentley
Copy link
Contributor Author

@george-zubrienko and @pawelwilczewski Building on #464 I added simple support for iterating over available dataclasses when deserializing objects withou __type specifiers.

george-zubrienko
george-zubrienko reviewed Sep 4, 2023
View reviewed changes
Copy link
Collaborator

@george-zubrienko george-zubrienko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thank you for contribution and nice to see people are actively collaborating! From what I see core.py it seems like your fork was behind master? It looks like the original commit from @pawelwilczewski ?

@idbentley
Copy link
Contributor Author

idbentley commented Sep 4, 2023 •
edited
Loading

I don't see what you mean @george-zubrienko . The logic change I added in core.py is adding an if check for each dataclass in the type_options, which catches potential decoding errors in order to test each dataclass, until a suitable one (or none is found).

@pawelwilczewski
Copy link
Contributor

@idbentley See what the final PR looked like. As per @george-zubrienko suggestion, I refactored the file a bit before finalising the PR.

@idbentley
Copy link
Contributor Author

Oh, I see! Sorry, I misunderstood your comment @george-zubrienko ! I've updated it to match recommended style!

@idbentley
Copy link
Contributor Author

Hi @george-zubrienko . Would love to see this PR get a review as it will enable a feature to be completed on my team.

I updated the style (as you asked) last week, and and rebased from master today.

Thanks for your consideration!

@george-zubrienko
Copy link
Collaborator

Hi @george-zubrienko . Would love to see this PR get a review as it will enable a feature to be completed on my team.

I updated the style (as you asked) last week, and and rebased from master today.

Thanks for your consideration!

Hello :) no worries I'll review/merge this week. Github also made a mess recently, which impacts our ability to release to PyPI, thus I cannot guarantee a PyPI release this week. Sep 14 is when GH engineering should provide a solution - or so I was told. Stay tuned for updates.

george-zubrienko
george-zubrienko previously approved these changes Sep 12, 2023
View reviewed changes
Copy link
Collaborator

@george-zubrienko george-zubrienko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor comment re exception catch

dataclasses_json/core.py Show resolved Hide resolved
dataclasses_json/core.py Show resolved Hide resolved
dataclasses_json/core.py Outdated Show resolved Hide resolved
@george-zubrienko
Copy link
Collaborator

Pleasant discussion, elaborate reasoning, all-around great PR. Time to merge!

@george-zubrienko george-zubrienko merged commit 5207d26 into lidatong:master Sep 12, 2023
9 checks passed
@idbentley
Copy link
Contributor Author

Great working with you @george-zubrienko ! Thanks for all the help.

@idbentley idbentley mentioned this pull request Oct 23, 2023
Closed
renovate bot referenced this pull request in ixm-one/pytest-cmake-presets Dec 10, 2023
@renovate @bruxisma
⬆️ Update dependency dataclasses-json to ^0.6.0 (#143)
fd8f2cd 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [dataclasses-json](https://togithub.com/lidatong/dataclasses-json)
([changelog](https://togithub.com/lidatong/dataclasses-json/releases)) |
`^0.5.7` -> `^0.6.0` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/dataclasses-json/0.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/dataclasses-json/0.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/dataclasses-json/0.5.9/0.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/dataclasses-json/0.5.9/0.6.3?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the warning logs for
more information.

---

### Release Notes

<details>
<summary>lidatong/dataclasses-json (dataclasses-json)</summary>

###
[`v0.6.3`](https://togithub.com/lidatong/dataclasses-json/releases/tag/v0.6.3)

[Compare
Source](https://togithub.com/lidatong/dataclasses-json/compare/v0.6.2...v0.6.3)

#### What's Changed

- Fixes catchall inheritance issue by
[@&#8203;jasonrock-a3](https://togithub.com/jasonrock-a3) in
[https://github.com/lidatong/dataclasses-json/pull/500](https://togithub.com/lidatong/dataclasses-json/pull/500)

#### New Contributors

- [@&#8203;jasonrock-a3](https://togithub.com/jasonrock-a3) made their
first contribution in
[https://github.com/lidatong/dataclasses-json/pull/500](https://togithub.com/lidatong/dataclasses-json/pull/500)

**Full Changelog**:
lidatong/dataclasses-json@v0.6.2...v0.6.3

###
[`v0.6.2`](https://togithub.com/lidatong/dataclasses-json/releases/tag/v0.6.2)

[Compare
Source](https://togithub.com/lidatong/dataclasses-json/compare/v0.6.1...v0.6.2)

#### What's Changed

- fix: allow using CatchAll with postponed evaluation of annotations by
[@&#8203;2ynn](https://togithub.com/2ynn) in
[https://github.com/lidatong/dataclasses-json/pull/490](https://togithub.com/lidatong/dataclasses-json/pull/490)
- Fix PEP 0673 before 3.11 by
[@&#8203;george-zubrienko](https://togithub.com/george-zubrienko) in
[https://github.com/lidatong/dataclasses-json/pull/487](https://togithub.com/lidatong/dataclasses-json/pull/487)
- fix mypy error when assigning to dataclass_json_config by
[@&#8203;MickaelBergem](https://togithub.com/MickaelBergem) in
[https://github.com/lidatong/dataclasses-json/pull/486](https://togithub.com/lidatong/dataclasses-json/pull/486)
- Fix an issue introduced with hetero tuple decode by
[@&#8203;george-zubrienko](https://togithub.com/george-zubrienko) in
[https://github.com/lidatong/dataclasses-json/pull/493](https://togithub.com/lidatong/dataclasses-json/pull/493)

#### New Contributors

- [@&#8203;2ynn](https://togithub.com/2ynn) made their first
contribution in
[https://github.com/lidatong/dataclasses-json/pull/490](https://togithub.com/lidatong/dataclasses-json/pull/490)
- [@&#8203;MickaelBergem](https://togithub.com/MickaelBergem) made their
first contribution in
[https://github.com/lidatong/dataclasses-json/pull/486](https://togithub.com/lidatong/dataclasses-json/pull/486)

**Full Changelog**:
lidatong/dataclasses-json@v0.6.1...v0.6.2

###
[`v0.6.1`](https://togithub.com/lidatong/dataclasses-json/releases/tag/v0.6.1)

[Compare
Source](https://togithub.com/lidatong/dataclasses-json/compare/v0.6.0...v0.6.1)

##### What's Changed

- Add links to make PyPI a better maintainer reference by
[@&#8203;pydanny](https://togithub.com/pydanny) in
[https://github.com/lidatong/dataclasses-json/pull/482](https://togithub.com/lidatong/dataclasses-json/pull/482)
- improve Union deserialization when "\__type" field specifier is not
present by [@&#8203;idbentley](https://togithub.com/idbentley) in
[https://github.com/lidatong/dataclasses-json/pull/478](https://togithub.com/lidatong/dataclasses-json/pull/478)

##### New Contributors

- [@&#8203;pydanny](https://togithub.com/pydanny) made their first
contribution in
[https://github.com/lidatong/dataclasses-json/pull/482](https://togithub.com/lidatong/dataclasses-json/pull/482)
- [@&#8203;idbentley](https://togithub.com/idbentley) made their first
contribution in
[https://github.com/lidatong/dataclasses-json/pull/478](https://togithub.com/lidatong/dataclasses-json/pull/478)

**Full Changelog**:
lidatong/dataclasses-json@v0.6.0...v0.6.1

###
[`v0.6.0`](https://togithub.com/lidatong/dataclasses-json/releases/tag/v0.6.0)

[Compare
Source](https://togithub.com/lidatong/dataclasses-json/compare/v0.5.14...v0.6.0)

##### What's Changed

- Improve dataclass_json and \_process_class type annotations by
[@&#8203;ringohoffman](https://togithub.com/ringohoffman) in
[https://github.com/lidatong/dataclasses-json/pull/475](https://togithub.com/lidatong/dataclasses-json/pull/475)
- Update Poetry version used for 3.7 test suite and change
Requires-Python boundary by
[@&#8203;george-zubrienko](https://togithub.com/george-zubrienko) in
[https://github.com/lidatong/dataclasses-json/pull/476](https://togithub.com/lidatong/dataclasses-json/pull/476)
- Fix for
[#&#8203;239](https://togithub.com/lidatong/dataclasses-json/issues/239):
Union inside List or Dict is not deserialized as the correspond… by
[@&#8203;pawelwilczewski](https://togithub.com/pawelwilczewski) in
[https://github.com/lidatong/dataclasses-json/pull/464](https://togithub.com/lidatong/dataclasses-json/pull/464)

Due to a behaviour change discovered in
[https://github.com/lidatong/dataclasses-json/issues/466](https://togithub.com/lidatong/dataclasses-json/issues/466)
and also as a matter of preparation for full deprecation of Py3.7, we
are bumping the minor version to 0.6.0. Most important change is that
since 0.5.9 builtins are coerced automatically without throwing an
exception. Please visit the issue for more info :)

##### New Contributors

- [@&#8203;ringohoffman](https://togithub.com/ringohoffman) made their
first contribution in
[https://github.com/lidatong/dataclasses-json/pull/475](https://togithub.com/lidatong/dataclasses-json/pull/475)
- [@&#8203;pawelwilczewski](https://togithub.com/pawelwilczewski) made
their first contribution in
[https://github.com/lidatong/dataclasses-json/pull/464](https://togithub.com/lidatong/dataclasses-json/pull/464)

**Full Changelog**:
lidatong/dataclasses-json@v0.5.15...v0.6.0

###
[`v0.5.14`](https://togithub.com/lidatong/dataclasses-json/releases/tag/v0.5.14)

[Compare
Source](https://togithub.com/lidatong/dataclasses-json/compare/v0.5.13...v0.5.14)

#### What's Changed

- Temporarily disable code coverage publish for fork PRs by
[@&#8203;george-zubrienko](https://togithub.com/george-zubrienko) in
[https://github.com/lidatong/dataclasses-json/pull/444](https://togithub.com/lidatong/dataclasses-json/pull/444)
- fix mashmallow fields.Tuple creation by
[@&#8203;healthmatrice](https://togithub.com/healthmatrice) in
[https://github.com/lidatong/dataclasses-json/pull/434](https://togithub.com/lidatong/dataclasses-json/pull/434)
- Allow the global config dictionary keys to also be Optional\[type] by
[@&#8203;sumnerevans](https://togithub.com/sumnerevans) in
[https://github.com/lidatong/dataclasses-json/pull/255](https://togithub.com/lidatong/dataclasses-json/pull/255)
- Fix global_config.mm_fields having no effect by
[@&#8203;sigmunau](https://togithub.com/sigmunau) in
[https://github.com/lidatong/dataclasses-json/pull/253](https://togithub.com/lidatong/dataclasses-json/pull/253)
- Fixes recursion bug related to enum flags by
[@&#8203;matt035343](https://togithub.com/matt035343) in
[https://github.com/lidatong/dataclasses-json/pull/447](https://togithub.com/lidatong/dataclasses-json/pull/447)
- Update Python version boundaries to include 3.12 by
[@&#8203;cdce8p](https://togithub.com/cdce8p) in
[https://github.com/lidatong/dataclasses-json/pull/449](https://togithub.com/lidatong/dataclasses-json/pull/449)

#### New Contributors

- [@&#8203;healthmatrice](https://togithub.com/healthmatrice) made their
first contribution in
[https://github.com/lidatong/dataclasses-json/pull/434](https://togithub.com/lidatong/dataclasses-json/pull/434)
- [@&#8203;sigmunau](https://togithub.com/sigmunau) made their first
contribution in
[https://github.com/lidatong/dataclasses-json/pull/253](https://togithub.com/lidatong/dataclasses-json/pull/253)
- [@&#8203;cdce8p](https://togithub.com/cdce8p) made their first
contribution in
[https://github.com/lidatong/dataclasses-json/pull/449](https://togithub.com/lidatong/dataclasses-json/pull/449)

**Full Changelog**:
lidatong/dataclasses-json@v0.5.13...v0.5.14

###
[`v0.5.13`](https://togithub.com/lidatong/dataclasses-json/releases/tag/v0.5.13)

[Compare
Source](https://togithub.com/lidatong/dataclasses-json/compare/v0.5.12...v0.5.13)

#### What's Changed

- Fixes bug related to Tuples defined with ellipsis by
[@&#8203;matt035343](https://togithub.com/matt035343) in
[https://github.com/lidatong/dataclasses-json/pull/440](https://togithub.com/lidatong/dataclasses-json/pull/440)
- Revert type hint in annotation call by
[@&#8203;george-zubrienko](https://togithub.com/george-zubrienko) in
[https://github.com/lidatong/dataclasses-json/pull/441](https://togithub.com/lidatong/dataclasses-json/pull/441)

**Full Changelog**:
lidatong/dataclasses-json@v0.5.12...v0.5.13

###
[`v0.5.12`](https://togithub.com/lidatong/dataclasses-json/releases/tag/v0.5.12)

[Compare
Source](https://togithub.com/lidatong/dataclasses-json/compare/v0.5.9...v0.5.12)

#### What's Changed

- Fix multiline scripts in CI by
[@&#8203;george-zubrienko](https://togithub.com/george-zubrienko) in
[https://github.com/lidatong/dataclasses-json/pull/439](https://togithub.com/lidatong/dataclasses-json/pull/439)

**Full Changelog**:
lidatong/dataclasses-json@v0.5.11...v0.5.12

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/ixm-one/pytest-cmake-presets).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi43OS4xIiwidXBkYXRlZEluVmVyIjoiMzcuODEuMyIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

---------

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Signed-off-by: Izzy Muerte <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Izzy Muerte <[email protected]>
@s4ke s4ke mentioned this pull request Jun 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@george-zubrienko george-zubrienko george-zubrienko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@idbentley @pawelwilczewski @george-zubrienko