Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Generic Dataclass Fields Mutation Bug (when using TypeAdapter) #7435

Merged
merged 5 commits into from Sep 14, 2023

Conversation

sydney-runkle
Copy link
Member

@sydney-runkle sydney-runkle commented Sep 13, 2023

Change Summary

Related issue number

fix #7430

Checklist

  • The pull request title is a good summary of the changes - it will be used in the changelog
  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@cloudflare-pages
Copy link

cloudflare-pages bot commented Sep 13, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: aa1d4c1
Status: ✅  Deploy successful!
Preview URL: https://3e55a1f6.pydantic-docs2.pages.dev
Branch Preview URL: https://dataclass-fields-mutation-bu.pydantic-docs2.pages.dev

View logs

@@ -65,7 +65,7 @@ def get_type_ref(type_: type[Any], args_override: tuple[type[Any], ...] | None =
when creating generic models without needing to create a concrete class.
"""
origin = type_
args = args_override or ()
args = type_.__args__ if isinstance(type_, _GenericAlias) else (args_override or ()) # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

@dmontagu, could you please explain why we changed this line?

I understand the value of the deepcopy on the model's fields in _generate_schema.py, but I don't quite understand the purpose of this line here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed this misbehavior — right now, it's not producing a type ref that reflects the generic parameters like it is supposed to. In particular, this means that in principle you could get two different schemas with the same ref but that should have different parameters. I don't know why that doesn't seem to be affecting the validation behavior, but I do think this is an improvement either way.

@sydney-runkle sydney-runkle marked this pull request as ready for review September 13, 2023 19:57
@dmontagu dmontagu enabled auto-merge (squash) September 13, 2023 22:02
@dmontagu dmontagu merged commit c6e2f89 into main Sep 14, 2023
50 checks passed
@dmontagu dmontagu deleted the dataclass-fields-mutation-bug branch September 14, 2023 03:47
@davidhewitt davidhewitt added the relnotes-fix Used for bugfixes. label Sep 21, 2023
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.

TypeAdapter fails to validate against generic dataclass when using different type parameters
3 participants