Skip to content

Conversation

sydney-runkle
Copy link
Contributor

@sydney-runkle sydney-runkle commented Sep 18, 2024

This was supposed to close #8271, but right now it just improves the associated error.

My current hunch is that we need to do an apply discriminators step at the end of json schema building, like we do for core schemas, if we want this to work 😭. Not sure how worth the investment that is, at the moment.

I'd also be open to more creative solutions for evaluating refs for which we have not yet generated the schemas. Probably a good conversation to have with @Viicos and @adriangb.

MRE, at the moment:

from enum import Enum
from typing import Annotated, Union, Literal, List

from pydantic import BaseModel as BaseModel, Field, TypeAdapter
from pydantic.fields import FieldInfo
import json
from pydantic._internal._core_utils import pretty_print_core_schema

class ItemType(str, Enum):
    ITEM1 = 'item1'
    ITEM2 = 'item2'

class CreateItem1(BaseModel):
    item_type: Annotated[Literal[ItemType.ITEM1], Field(alias='type')]
    id: int

class CreateItem2(BaseModel):
    item_type: Annotated[Literal[ItemType.ITEM2], Field(alias='type')]
    id: int

class CreateObjectDto(BaseModel):
    id: int
    items: List[
        Annotated[
            Union[
                CreateItem1,
                CreateItem2,
            ],
            Field(discriminator='item_type'),
        ]
    ]

ta = TypeAdapter(
    Annotated[CreateObjectDto, FieldInfo(examples=[{'id': 1, 'items': [{'id': 3, 'type': 'ITEM1'}]}])]
)

print(json.dumps(ta.json_schema(), indent=2))
print(pretty_print_core_schema(ta.core_schema))

Copy link

codspeed-hq bot commented Sep 18, 2024

CodSpeed Performance Report

Merging #10440 will not alter performance

Comparing disc-fix-alias (113414a) with main (eda3eb4)

Summary

✅ 38 untouched benchmarks

Copy link
Contributor

github-actions bot commented Sep 18, 2024

Coverage report

This PR does not seem to contain any modification to coverable code.

Copy link

cloudflare-workers-and-pages bot commented Oct 3, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 113414a
Status: ✅  Deploy successful!
Preview URL: https://b1d5d5ab.pydantic-docs.pages.dev
Branch Preview URL: https://disc-fix-alias.pydantic-docs.pages.dev

View logs

@@ -466,7 +466,6 @@ def populate_defs(core_schema: CoreSchema, json_schema: JsonSchemaValue) -> Json
core_ref = CoreRef(core_schema['ref']) # type: ignore[typeddict-item]
defs_ref, ref_json_schema = self.get_cache_defs_ref_schema(core_ref)
json_ref = JsonRef(ref_json_schema['$ref'])
self.json_to_defs_refs[json_ref] = defs_ref
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done in the self.get_cache_defs_ref_schema(core_ref) call, so no reason to do that here.

while '$ref' in choice:
assert isinstance(choice['$ref'], str)
choice = self.get_schema_from_definitions(JsonRef(choice['$ref'])) or {}
choice = self.resolve_schema_to_update(choice)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function handles the above removed logic more gracefully, with an intuitive error.

@sydney-runkle sydney-runkle changed the title Working on fixing json schema for discriminators Improve error handling for in-evaluable refs for discriminator application Oct 3, 2024
@sydney-runkle sydney-runkle requested a review from Viicos October 3, 2024 20:28
Copy link
Member

@Viicos Viicos left a comment

Choose a reason for hiding this comment

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

I'll try to do a deep dive in the JSON Schema generation process to see what this is about, I'm guessing it is indeed pretty similar to the core schema gen process

@sydney-runkle
Copy link
Contributor Author

Note from my conversation with @adriangb - we could alternatively do a first pass to collect all defs

@sydney-runkle sydney-runkle enabled auto-merge (squash) October 7, 2024 13:26
@sydney-runkle sydney-runkle merged commit 4e71e66 into main Oct 7, 2024
58 checks passed
@sydney-runkle sydney-runkle deleted the disc-fix-alias branch October 7, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes-fix Used for bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenAPI discriminator field disapeared with pydantic v2.5
2 participants