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

Added fix for signature of inherited dataclass #7925

Merged

Conversation

howsunjow
Copy link
Contributor

@howsunjow howsunjow commented Oct 25, 2023

Change Summary

Added fix for signature of inherited dataclass when using the pydantic dataclass decorator.

There is a bug in the following code

from pydantic.dataclasses import dataclass

@dataclass
class A:
    a: int

@dataclass
class B(A):
    b: int

where the signature of the class B does not include the subclass (B) fields (see issue #7906 )

  • Used generate_model_signature to create a temporary signature before running generate_dataclass_signature
  • Added unit test for fix

Related issue number

fix #7906

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

please review

@sydney-runkle sydney-runkle added the relnotes-fix Used for bugfixes. label Oct 25, 2023
Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Hi @howsunjow,

Thanks so much for your contribution. The test that you've written looks great.

I left one comment regarding changes that we will likely want to make to the generate_dataclass_signature function. I think this would be a cleaner approach. I certainly wouldn't be opposed to centralizing some of the shared logic between the generate_dataclass_signature function and the generate_model_signature function once we figure out the commonalities that they share.

pydantic/_internal/_dataclasses.py Outdated Show resolved Hide resolved
Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

@howsunjow,

I left some very minor nit picks.

This looks much better - thanks for the change! I'd still like to reduce some of the redundancy that we have across the generate_model_signature and generate_dataclass_signature functions. Specifically, I think we can centralize some of the shared logic + use that in both cases.

Would you like to take a stab at that? If not, no worries, I'd be happy to take that on.

Great work 🌟, and thanks again for your contribution!

pydantic/_internal/_dataclasses.py Outdated Show resolved Hide resolved
pydantic/_internal/_dataclasses.py Outdated Show resolved Hide resolved
@howsunjow
Copy link
Contributor Author

@howsunjow,

I left some very minor nit picks.

This looks much better - thanks for the change! I'd still like to reduce some of the redundancy that we have across the generate_model_signature and generate_dataclass_signature functions. Specifically, I think we can centralize some of the shared logic + use that in both cases.

Would you like to take a stab at that? If not, no worries, I'd be happy to take that on.

Great work 🌟, and thanks again for your contribution!

I've made some additional changes to centralize the the shared signature generation logic.

Copy link
Member

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

@howsunjow Nice! Thanks for refactoring to take advantage of shared code.

Merging now 😄. Thanks for your contribution ⭐!

@sydney-runkle sydney-runkle merged commit 0ca1cf2 into pydantic:main Oct 27, 2023
59 checks passed
@howsunjow howsunjow deleted the bugfix/inherited_dataclass_signature branch May 4, 2024 16:49
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.

Signature of inherited dataclass does not include subclass field
3 participants