Consolidate schema definitions logic in the _Definitions class#11208
Consolidate schema definitions logic in the _Definitions class#11208
_Definitions class#11208Conversation
fcd4818 to
5de851f
Compare
Deploying pydantic-docs with
|
| Latest commit: |
c388c19
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://506c3228.pydantic-docs.pages.dev |
| Branch Preview URL: | https://definitions-improvements.pydantic-docs.pages.dev |
| self.seen: set[str] = set() | ||
| self.definitions: dict[str, core_schema.CoreSchema] = {} | ||
| self._recursively_seen = set() | ||
| self._definitions = {} |
There was a problem hiding this comment.
@MarkusSintonen, in your PR, you made a distinction between definitions stored from create_reference_to_schema and definitions coming from unpack_definitions (in your PR, as _unpacked_definitions), i.e. definitions potentially coming from the cached attribute of Pydantic models.
What was the reason to do so?
CodSpeed Performance ReportMerging #11208 will not alter performanceComparing Summary
|
| # if schema['type'] == 'definition-ref': | ||
| # return core_schema.definition_reference_schema(schema_ref=schema['schema_ref']) |
There was a problem hiding this comment.
@MarkusSintonen, this is an addition in your PR. Seems like test passes without it. Do you remember why this might be required? I'm assuming this is part of #10655, where 'definition-ref' schemas are inlined in place, and thus this might some kind of safety copy?
There was a problem hiding this comment.
I'm removing it from this PR, as if it needs to be included, this will be in the schema walking refactor PR.
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
5de851f to
e3a19eb
Compare
e3a19eb to
b6e5c54
Compare
sydney-runkle
left a comment
There was a problem hiding this comment.
I like this refactor overall, I think it does simplify things and make our already confusing defs/refs management logic a bit more clear.
Thanks for adding those docstrings as well.
Some common patterns are defined as methods to avoid duplication (e.g. `create_reference_to_schema`, used to store a schema as a definition and returns a `'definition-reference'` schema). A couple existing methods on the `GenerateSchema` class are moved (and renamed to better describe what they do) to the `_Definitions` class. Proper docstrings are added to attribute and methods. Type hints are tweaked when necessary.
b6e5c54 to
79bec86
Compare
sydney-runkle
left a comment
There was a problem hiding this comment.
I'm happy with this given the re-namings, thanks!
Some common patterns are defined as methods to avoid duplication (e.g.
create_reference_to_schema, used to store a schema as a definition and returns a'definition-reference'schema). A couple existing methods on theGenerateSchemaclass are moved (and renamed to better describe what they do) to the_Definitionsclass.Proper docstrings are added to attribute and methods.
Type hints are tweaked when necessary.
This is meant to be a simplified changeset of #10887, only with the necessary changes present.
Change Summary
Related issue number
Checklist