Skip to content

Fix tagged unions multiple processing in submodels#6340

Merged
adriangb merged 3 commits intopydantic:mainfrom
vadim-su:fix_tagged_unions_in_submodels
Jul 1, 2023
Merged

Fix tagged unions multiple processing in submodels#6340
adriangb merged 3 commits intopydantic:mainfrom
vadim-su:fix_tagged_unions_in_submodels

Conversation

@vadim-su
Copy link
Copy Markdown
Contributor

@vadim-su vadim-su commented Jul 1, 2023 •
edited by pydantic-hooky Bot
Loading

Change Summary

In some cases like that:

import enum
from typing import Annotated, Literal

from pydantic import BaseModel, Field


class ComponentType(enum.IntEnum):
        SomeComponent1 = 1
        SomeComponent2 = 2

class SomeComponent1(BaseModel):
    type: Literal[ComponentType.SomeComponent1] = ComponentType.SomeComponent1
    
class SomeComponent2(BaseModel):
    type: Literal[ComponentType.SomeComponent2] = ComponentType.SomeComponent2

Component = Annotated[SomeComponent1|SomeComponent2, Field(discriminator='type')]

class TestModel1(BaseModel):
    components: list[Component]
    
class TestModel2(BaseModel):
    components: list[Component]
    
class TestModel3(BaseModel):
    submodels: TestModel1|TestModel2

apply_discriminators function could process tagged-unions because the tagged-union contains discriminator, but doesn't take that the schema was processed in another model. I excluded tagged-unions from the recursive processing.

I'm not sure about the place of the fix. Possibly a better place for this is _ApplyInferredDiscriminator.apply

pydantic/pydantic/_internal/_discriminated_union.py

Lines 154 to 160 in 15d5e0b

self.definitions.update(collect_definitions(schema))
assert not self._used
schema = self._apply_to_root(schema)
if self._should_be_nullable and not self._is_nullable:
schema = core_schema.nullable_schema(schema)
self._used = True
return schema

Related issue number

Fix #6339

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @lig

@vadim-su
Copy link
Copy Markdown
Contributor Author

vadim-su commented Jul 1, 2023

please review

@jooola jooola mentioned this pull request Jul 1, 2023
Merged
1 task
dmontagu
dmontagu reviewed Jul 1, 2023
View reviewed changes
Comment thread tests/test_discriminated_union.py
union_model: UnionModel

class TestModel(BaseModel):
submodel: Union[SubModel1, SubModel2]
Copy link
Copy Markdown
Contributor

@dmontagu dmontagu Jul 1, 2023

Choose a reason for hiding this comment

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

@suharnikov any chance you could add a bit of validation here just to make sure it actually behaves correctly during validation, not just that it stops erroring while building the schema

Copy link
Copy Markdown
Member

@adriangb adriangb Jul 1, 2023

Choose a reason for hiding this comment

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

Also a json schema check would be good

@adriangb adriangb enabled auto-merge (squash) July 1, 2023 15:53
@adriangb adriangb merged commit 1943e9d into pydantic:main Jul 1, 2023
@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented Jul 1, 2023

We need to start requiring change files.

@vadim-su
Copy link
Copy Markdown
Contributor Author

vadim-su commented Jul 1, 2023

I missed all the fun 😅

@vadim-su vadim-su deleted the fix_tagged_unions_in_submodels branch July 1, 2023 20:56
@a10y
Copy link
Copy Markdown

a10y commented Jul 4, 2023 •
edited
Loading

@samuelcolvin / @dmontagu Is there going to be a minor release sometime soon with this fix? I use nested discriminated unions pretty extensively in my API so this is currently blocking my upgrade to v2

EDIT: This is now in 2.0.3, thanks all!

@a10y a10y mentioned this pull request Jul 4, 2023
Merged
1 task
a10y added a commit to IntrinsicLabsAI/intrinsic-model-server that referenced this pull request Jul 6, 2023
@a10y
Pydantic v2 (#99)
da89af7 
## Bump to Pydantic v2

This PR brings the repo onto Pydantic v 2.0.2 and FastAPI v
0.100.0-beta3 (0.100.0 is expected to cut by EOW and this is the last
beta release).

* Rework how we do custom tagged union types
* The new paradigm for this involves extending a `RootModel` generic
type from pydantic, which I find a bit gross but it is the new supported
way to do this so 🤷
* Use all of the new `model_` methods that were renamed for AFAICT no
particular reason
* Bump the FE generated types to use the new SemVer type alias in the
FE.

- [X] Blocked until we get a new release
pydantic/pydantic#6340 (comment)
@Foxfire1st Foxfire1st mentioned this pull request Oct 14, 2025
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmontagu dmontagu dmontagu left review comments

@adriangb adriangb adriangb approved these changes

Assignees

@lig lig

Labels

ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TypeAdapter doesn't work properly with tagged-unions

6 participants

@vadim-su @samuelcolvin @a10y @adriangb @dmontagu @lig