Fix dataclass asdict by benderl · Pull Request #3886 · openWB/core

benderl · 2026-09-03T10:05:11Z

No description provided.

Copilot

🟡 Changes recommended

Valid tuple or None states returned by __getstate__() are not handled correctly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates dataclass_utils.asdict to respect custom object serialization state.

Changes:

Uses __getstate__() when available.
Adds tests for filtered custom state.

File summaries

File	Description
`packages/dataclass_utils/_dataclass_asdict.py`	Adds custom-state serialization, but incorrectly assumes `__getstate__()` returns a mapping.
`packages/dataclass_utils/_dataclass_asdict_test.py`	Tests custom-state serialization.

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        state = getattr(value, "__getstate__", None)
+        value = state() if state is not None else vars(cast(object, value))


Copilot

🟡 Changes recommended

The inherited object.__getstate__ invocation causes a critical Python 3.11+ regression.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

packages/dataclass_utils/_dataclass_asdict.py:35

Returning the original object here does not make it an AsDictValue; every JSON-native leaf type has already been handled above, so values reaching this branch (for example object() or a lock) remain non-serializable. This also defers the failure to callers such as Pub.pub, which run json.dumps (packages/helpermodules/pub.py:20), instead of producing the function's declared JSON-like result. Preserve the TypeError (or explicitly convert supported leaf types) rather than hiding it with a cast.

        try:
            value = vars(cast(object, value))
        except TypeError:
            return cast(AsDictValue, value)

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

The critical __getstate__ invocation bug must be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

+        default_getstate = getattr(object, "__getstate__", None)
+        if callable(default_getstate) and getattr(type(value), "__getstate__", None) is not default_getstate:
+            return asdict(default_getstate())


Copilot

🟡 Changes recommended

The fallback can return unsupported, non-serializable objects instead of failing explicitly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

+        try:
+            value = vars(cast(object, value))
+        except TypeError:
+            return cast(AsDictValue, value)


Copilot

🟡 Changes recommended

Dataclass fields must be read only once during serialization.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Balanced

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

🟢 Approval recommended

All reviewed changes are covered with no unresolved issues.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Balanced

* fix dataclass asdict * review * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix review * fix * review * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: LKuemmel <lena.kuemmel@openwb.de> Co-authored-by: LKuemmel <76958050+LKuemmel@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

fix dataclass asdict

908cd70

benderl requested a review from LKuemmel September 3, 2026 10:05

benderl added the bug Something isn't working label Sep 3, 2026

benderl changed the base branch from master to feature_chargemode_enums September 3, 2026 10:05

LKuemmel requested a balanced review from Copilot September 7, 2026 06:49

Copilot started reviewing on behalf of LKuemmel September 7, 2026 06:50 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread packages/dataclass_utils/_dataclass_asdict.py Outdated

Comment on lines +27 to +28

state = getattr(value, "__getstate__", None)

value = state() if state is not None else vars(cast(object, value))

review

7e56260

LKuemmel requested a balanced review from Copilot September 7, 2026 09:35

Copilot started reviewing on behalf of LKuemmel September 7, 2026 09:35 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread packages/dataclass_utils/_dataclass_asdict.py

Potential fix for pull request finding

10242f7

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

LKuemmel approved these changes Sep 7, 2026

View reviewed changes

fix review

a849598

LKuemmel requested a balanced review from Copilot September 7, 2026 09:56

Copilot started reviewing on behalf of LKuemmel September 7, 2026 09:56 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread packages/dataclass_utils/_dataclass_asdict.py Outdated

Comment on lines +29 to +31

default_getstate = getattr(object, "__getstate__", None)

if callable(default_getstate) and getattr(type(value), "__getstate__", None) is not default_getstate:

return asdict(default_getstate())

fix

1c458cb

LKuemmel requested a balanced review from Copilot September 7, 2026 10:05

Copilot started reviewing on behalf of LKuemmel September 7, 2026 10:05 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread packages/dataclass_utils/_dataclass_asdict.py Outdated

Comment on lines +34 to +37

try:

value = vars(cast(object, value))

except TypeError:

return cast(AsDictValue, value)

review

ded83d5

LKuemmel requested a balanced review from Copilot September 7, 2026 12:46

Copilot started reviewing on behalf of LKuemmel September 7, 2026 12:46 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

Comment thread packages/dataclass_utils/_dataclass_asdict.py Outdated

Potential fix for pull request finding

73dea56

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

LKuemmel requested a balanced review from Copilot September 7, 2026 12:55

Copilot started reviewing on behalf of LKuemmel September 7, 2026 12:56 View session

Copilot AI reviewed Sep 7, 2026

View reviewed changes

LKuemmel merged commit 1ee158d into openWB:feature_chargemode_enums Sep 7, 2026
1 check passed

Navigation Menu

Uh oh!

Fix dataclass asdict - #3886

Fix dataclass asdict#3886
LKuemmel merged 7 commits into
openWB:feature_chargemode_enumsfrom
benderl:fix-dataclass-asdict

benderl commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		state = getattr(value, "__getstate__", None)
		value = state() if state is not None else vars(cast(object, value))

Uh oh!

Conversation

benderl commented Sep 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants