[python] Preserve nested union container values - #25005
Closed
markstuart-oai wants to merge 1 commit into
Closed
markstuart-oai wants to merge 1 commit into
markstuart-oai wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A Python union containing an array/map of another generated union can silently lose valid data. For example, with
Message.content = oneOf(string, array<Part>)andPart = oneOf(Text, Image),Message.from_dict({"content": [{"text": "x"}]})serializes the content as[null]. The outer union lets Pydantic construct the inner wrapper without selecting itsactual_instance.to_json()also exposes wrapper fields for explicitly constructed nested values.Reproduced on 7.25.0 and current master
6f77724ce5d4a9d61ccb91fde60d80f1c38b0bed, using Python 3.12.13 and Pydantic 2.12.5/2.13.5. The reduction needs no OpenAI extensions, seed, nullable fields, or OpenAPI 3.1. Related: #22261 and OpenAI's serialization report; this is an independent reproduction, not an identification of that reporter's unknown toolchain.Change
from_dict, then retain the existing Pydantic container validation and union matching rules.from_dictwould change omitted nullable fields into explicit nulls; regression coverage prevents that.to_dict/to_json, without depending onApiClient.actual_instance, and legacy public/wire dictionary behavior. The intended observable correction is that modernto_dictreturns nested data instead of wrapper objects. This is not a general rewrite of Pydantic model validation and does not fix the separatepython-pydantic-v1generator.Four implementation files and two fixture/test files are authored changes. The remaining files are regenerated samples, including the two Pydantic-v1 samples that share the fixture.
Validation
PythonClientCodegenTest: 90 passed; CLI build passed.bin/generate-samples.sh.Native command:
./mvnw -pl modules/openapi-generator-cli -am package -DskipTests=false -Dtest=PythonClientCodegenTest -Dsurefire.failIfNoSpecifiedTests=false -Dmaven.javadoc.skip=true(optional remote build cache and build-scan upload disabled locally). No full live-Petstore integration claim.PR checklist
Summary by cubic
Fixes Python union models losing data when they contain arrays or maps of other generated unions:
from_dictno longer produces[null]for nested values, andto_dict/to_jsonreturn the nested data instead of wrapper objects.from_dict, preserving Pydantic container validation and union matching rules.to_dict/to_jsonwithout depending onApiClient; constructors, class names, andactual_instancebehavior are unchanged.python-pydantic-v1generator is not affected.Written for commit 2b275f9. Summary will update on new commits.