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
Added fix for signature of inherited dataclass #7925
Conversation
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!
…ass and BaseModel.
I've made some additional changes to centralize the the shared signature generation logic. |
There was a problem hiding this 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 ⭐!
Change Summary
Added fix for signature of inherited dataclass when using the pydantic dataclass decorator.
There is a bug in the following code
where the signature of the class B does not include the subclass (B) fields (see issue #7906 )
Related issue number
fix #7906
Checklist
please review