Skip to content

Added fix for signature of inherited dataclass#7925

Merged
sydney-runkle merged 3 commits intopydantic:mainfrom
howsunjow:bugfix/inherited_dataclass_signature
Oct 27, 2023
Merged

Added fix for signature of inherited dataclass#7925
sydney-runkle merged 3 commits intopydantic:mainfrom
howsunjow:bugfix/inherited_dataclass_signature

Conversation

@howsunjow
Copy link
Copy Markdown
Contributor

@howsunjow howsunjow commented Oct 25, 2023 •
edited
Loading

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
sydney-runkle
sydney-runkle suggested changes Oct 25, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

Comment thread pydantic/_internal/_dataclasses.py Outdated
@pydantic-hooky pydantic-hooky Bot added awaiting author revision awaiting changes from the PR author labels Oct 25, 2023
sydney-runkle
sydney-runkle suggested changes Oct 26, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@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!

Comment thread pydantic/_internal/_dataclasses.py Outdated
Comment thread pydantic/_internal/_dataclasses.py Outdated
@howsunjow
Copy link
Copy Markdown
Contributor Author

howsunjow commented Oct 26, 2023

@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.

sydney-runkle
sydney-runkle approved these changes Oct 27, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@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
@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

Reviewers

1 more reviewer

@sydney-runkle sydney-runkle sydney-runkle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@howsunjow howsunjow

Labels

awaiting author revision awaiting changes from the PR author relnotes-fix Used for bugfixes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Signature of inherited dataclass does not include subclass field

3 participants

@howsunjow @sydney-runkle @hjow1