Skip to content

Conversation

dmontagu
Copy link
Contributor

@dmontagu dmontagu commented Jul 25, 2023

Closes #6742

Currently, once you have rebuilt a model, the field annotations aren't updated from the original forward refs; as a result, when you declare subclasses in new modules, the same forward ref gets re-evaluated, even if the original model was rebuilt with a specific annotation in use.

This PR changes things so that the FieldInfos in cls.model_fields do get updated to resolve the ForwardRefs, ensuring that the resolved fields retain their resolved type even when new subclasses are defined in different modules.

I still need to add a test along the lines of what is described in #6742 (comment), but I was able to confirm that files written in that way seem to work with these changes. Before I attempt to add such tests though, I was hoping to get confirmation that this approach would be accepted.

Selected Reviewer: @samuelcolvin

@@ -338,6 +336,34 @@ def replace_types(type_: Any, type_map: Mapping[Any, Any] | None) -> Any:
return type_map.get(type_, type_)


def has_instance_in_type(type_: Any, isinstance_target: Any) -> bool:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although this doesn't really relate to pydantic generics functionality, I put this function in _generics.py because it is very closely related in implementation to _generics.replace_types, and because the interesting parts of the logic are mostly related to "walking" generic types.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 25, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 70986db
Status: ✅  Deploy successful!
Preview URL: https://c5a0568f.pydantic-docs2.pages.dev
Branch Preview URL: https://forwardref-field-updating.pydantic-docs2.pages.dev

View logs

@dmontagu
Copy link
Contributor Author

please review

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

LGTM

@samuelcolvin samuelcolvin merged commit 197bf18 into main Jul 25, 2023
@samuelcolvin samuelcolvin deleted the forwardref-field-updating branch July 25, 2023 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect ForwardRef class lookup in inherited models
2 participants