Skip to content

Fix enum with tuple values deserialization#765

Open
luxShrine wants to merge 2 commits into
yukinarit:mainfrom
luxShrine:fix-non-simple-deserialization
Open

Fix enum with tuple values deserialization#765
luxShrine wants to merge 2 commits into
yukinarit:mainfrom
luxShrine:fix-non-simple-deserialization

Conversation

@luxShrine

Copy link
Copy Markdown

First noted in (#585), when a tuple field of an enum was serialized into a list, it would not be deserialized into a tuple. This fix aims to patch that gap, by making a minimal change to the enum deserializing helper function, simply checking if the enum field is a tuple and converting it from a list if so.

This is a small patch, and in the original discussion it was noted that a more robust system would be to have a setup that would generate "(de)serialize functions for enum", not unlike what is done currently for Union. This patch for example only covers a single depth of tuples: ("a", ("b", "c")) serializes as ["a", ["b", "c"]] and deserializes as ("a", ["b", "c"]) which would fail to properly align with the original enum value. However, the value in making a small fix like this seemed worthwhile to patch, and leaves the case of more complex fields to the more substantial project of generating deserialization functions for enums.

luxshrine added 2 commits June 17, 2026 16:21
When serializing an enum of tuple values, the tuples are converted into
lists. To deserialize back into the expected enum field, these would
previously fail as the types did not match. This change now implements a
check by reusing the member_value_type of a sequence in the
deserialize_enum function, and will properly apply the conversion to a
tuple if found.
Appended two tuple enum tests to the existing deserialize enum helper
test, to ensure the basic deserialization of the type is done correctly.
Added a new test to ensure that the original issue (yukinarit#585) is resolved by
testing round trip (de)serialization works with various container types.
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.20%. Comparing base (f09d503) to head (d4aeefd).
⚠️ Report is 80 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #765      +/-   ##
==========================================
+ Coverage   90.83%   91.20%   +0.37%     
==========================================
  Files          13       13              
  Lines        2040     2389     +349     
  Branches      370      475     +105     
==========================================
+ Hits         1853     2179     +326     
- Misses        124      138      +14     
- Partials       63       72       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gaoflow

gaoflow commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

The failing Check formatting job is Ruff C901 on serde/de.py::from_obj, which is unchanged in this PR. I reproduced the same failure on current main with:

UV_FROZEN=1 uv run --frozen ruff check --force-exclude serde/de.py

I opened #767 to fix that main-branch lint regression separately. That PR is now green, so this PR should not need a business-logic change for the formatting failure.

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