fix(models): reject polymorphic_serialization on pydantic v1 - #2741
Open
ckarnell wants to merge 1 commit into
Open
fix(models): reject polymorphic_serialization on pydantic v1#2741ckarnell wants to merge 1 commit into
ckarnell wants to merge 1 commit into
Conversation
The pydantic v1 compatibility shim raises a clear ValueError for every v2-only option it cannot honour: mode, round_trip, warnings, context, fallback, serialize_as_any and exclude_computed_fields. polymorphic_serialization was the one exception, present in the signature of both model_dump and model_dump_json and never read, so a v1 caller passing it got no error and no effect. Adds the guard in the existing style next to the exclude_computed_fields one, in both methods, plus a test that fails without the change.
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
1 participant
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.
On pydantic v1,
BaseModelaliases the v2 method names and raises a clearValueErrorfor every option v1 cannot honour:mode,round_trip,warnings,context,fallback,serialize_as_any,exclude_computed_fields.polymorphic_serializationis the one exception. It sits in the signature of bothmodel_dumpandmodel_dump_jsonand is never read.Measured, not eyeballed: of the 14 keyword-only parameters on
model_dump, 13 are either forwarded tosuper().dict()or guarded by a raise.polymorphic_serializationis neither. Same inmodel_dump_json, 14 of 15.So a v1 user who passes it gets no error and no effect, and the call looks like it worked. Every sibling in the same function tells them otherwise.
The fix adds the guard in the existing style, next to the
exclude_computed_fieldsone, in both methods. Four lines.Verified under pydantic 1.10.26, which is where the shim is actually live. The new test fails without the change with
DID NOT RAISE ValueErrorand passes with it.tests/test_models.pyis 46 passed and 13 skipped on v1, and 59 passed on pydantic v2 where the shim is inactive, so nothing regresses on the path most users are on.ruff checkandruff format --checkare clean at the repo's own line length.Note that this is low severity, it needs a v1 user who reaches for a fairly new pydantic option, and the consequence is a silently ignored argument, not wrong output. The argument for fixing it is consistency: thirteen neighbours already do the right thing.
I didn't check whether the generator would reintroduce it. CONTRIBUTING says manual modifications persist between generations, which is why I sent a patch instead of an issue, but I have not seen a regeneration happen.