Skip to content

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

@pydantic-hooky pydantic-hooky bot added awaiting author revision awaiting changes from the PR author labels Oct 25, 2023
Copy link
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!

@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
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
Labels
awaiting author revision awaiting changes from the PR author relnotes-fix Used for bugfixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signature of inherited dataclass does not include subclass field
3 participants