Move some field logic out of the GenerateSchema class
#11733
Merged
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
CodSpeed Performance ReportMerging #11733 will not alter performanceComparing Summary
|
Viicos
force-pushed
the
reorganize-fields-collection
branch
from
14c90b2 to
5790c25
Compare
Deploying pydantic-docs with
|
| Latest commit: |
da78135
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2b47d3ae.pydantic-docs.pages.dev |
| Branch Preview URL: | https://reorganize-fields-collection.pydantic-docs.pages.dev |
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Viicos
added
relnotes-change
Viicos
commented
Apr 11, 2025
| @@ -1537,7 +1415,8 @@ def _typed_dict_schema(self, typed_dict_cls: Any, origin: Any) -> core_schema.Co | |||
| and field_name in field_docstrings | |||
| ): | |||
| field_info.description = field_docstrings[field_name] | |||
| self._apply_field_title_generator_to_field_info(self._config_wrapper, field_info, field_name) | |||
There was a problem hiding this comment.
This was actually called twice, here in the _typed_dict_schema() and later in _common_field_schema().
Viicos
force-pushed
the
reorganize-fields-collection
branch
2 times, most recently
from
6f8584a to
c89bcbf
Compare
Also rewrite the warning message, switch from `NameError` (meant for names not found) to a `ValueError`, and improve coverage.
There's no caller of the relevant functions that are providing None, so we can safely make `config_wrapper` a required parameter.
This is where most of the work is being done. A new `update_field_from_config()` function is defined and can be used to update the `FieldInfo` instances from the configuration of the model/class the field belongs to. Currently, this function will update the field instances by applying alias and title generators. Two reasons to move this logic out: - Alleviate the `GenerateSchema` class, which is currently a big monster. - Apply these updates as early as possible, when models are being created. This way (assuming annotations where resolved), users can safely inspect `FieldInfo.alias/title` even if `defer_build=True`.
Viicos
force-pushed
the
reorganize-fields-collection
branch
from
c89bcbf to
9e3f6a1
Compare
This is done through the `DecoratorInfos` via a new method. Ideally, we could also pass the `config_wrapper` in the existing `build()` method, but this method is recursive so it isn't straightforward. This has the unfortunate consequence of having to create the `DecoratorInfos` in two steps/calls, which is error prone. Perhaps in the future `build()` can be refactored to avoid the recursive calls.
Viicos
force-pushed
the
reorganize-fields-collection
branch
from
9e3f6a1 to
da78135
Compare
Viicos
added
the
third-party-tests
There was a problem hiding this comment.
Pull Request Overview
This PR refactors field‐related logic by extracting parts of the alias and title generation from the GenerateSchema class into dedicated helper functions, thereby improving modularity and clarity. Key changes include:
- Consistent updates to warning messages in tests and error messages in schema generation.
- Introduction of new helper functions (e.g. get_first_not_none, update_field_from_config) to centralize logic previously embedded in GenerateSchema.
- Propagation of configuration changes in fields, decorators, and dataclass handling.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_main.py & test_create_model.py | Updated warning messages for protected namespace conflicts. |
| tests/test_computed_fields.py & test_aliases.py | Added tests for alias generator behavior when deferring build. |
| pydantic/fields.py | Refactored type hints and moved some legacy alias logic. |
| pydantic/_internal/_generate_schema.py | Removed in‐module alias generation and added config_wrapper to field rebuild calls. |
| pydantic/_internal/_fields.py | Centralized protected namespace checking using a new helper. |
| pydantic/_internal/_decorators.py | Introduced update_from_config to update computed field info. |
| pydantic/dataclasses.py | Updated dataclass processing with explicit config updates. |
Comments suppressed due to low confidence (3)
pydantic/_internal/_generate_schema.py:743
- Ensure that passing config_wrapper to rebuild_model_fields correctly propagates the new alias and title generator settings, given that legacy alias generation logic has been removed from this module.
fields = rebuild_model_fields(
cls,
+ config_wrapper=self._config_wrapper,
pydantic/_internal/_decorators.py:518
- Ensure that the new update_from_config method is covered by unit tests to confirm that computed field info is correctly updated from the config.
def update_from_config(self, config_wrapper: ConfigWrapper) -> None:
pydantic/dataclasses.py:219
- Verify that adding the call to decorators.update_from_config(config_wrapper) in the dataclass processing pipeline does not introduce any unintended side effects; ensure corresponding tests validate this integration.
decorators.update_from_config(config_wrapper)
|
@Viicos Reading the commit messages, the changes seem reasonable, and the fact that CI is green gives me confidence, but since I haven't actually read each line of the diff I thought I'd let Copilot do that :D Let's see if it has anything to add... |
DouweM
approved these changes
Apr 25, 2025
|
@Viicos Good to go unless you still had something you wanted to add. |
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.
Change Summary
This can be reviewed commit per commit, and each commit message gives the necessary context to understand the motivation.
Related issue number
Checklist