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 error with empty dataclass inheritance #4576

Closed
wants to merge 2 commits into from
Closed

Fix error with empty dataclass inheritance #4576

wants to merge 2 commits into from

Conversation

dolfinus
Copy link
Contributor

@dolfinus dolfinus commented Oct 1, 2022

Fix @pydantic.dataclasses.dataclass() syntax failure on inheriting dataclass without changing fields list.

Closes #4565

@samuelcolvin
Copy link
Member

@PrettyWood could you take a look at this?

@samuelcolvin samuelcolvin mentioned this pull request Oct 5, 2022
8 tasks
@PrettyWood
Copy link
Member

Does it fix all new tests related to dataclasses or just one?

@samuelcolvin
Copy link
Member

No idea yet, @dolfinus please can you update the issue description to auto-close related issues.

Also, can you check if this fixes any other issues from #4552.

@samuelcolvin samuelcolvin changed the title Fix issue #4565 Fix error with empty dataclass inheritance Oct 5, 2022
@samuelcolvin
Copy link
Member

Also @dolfinus please include a descriptive PR title and don't include issue ids in titles.

@dolfinus
Copy link
Contributor Author

dolfinus commented Oct 5, 2022

This PR fixes only #4565. Other issues from #4552 are still present

@dolfinus
Copy link
Contributor Author

Failing test for FastAPI is not linked to this PR, the same check is failing on 1.10x-fixes branch

@dolfinus
Copy link
Contributor Author

Any updates?

@samuelcolvin
Copy link
Member

This seems fine to me, @PrettyWood are you happy to accept this?

@samuelcolvin
Copy link
Member

This affects the same line discussed on #4498, but doesn't appear to fix the problem.

@PrettyWood do you have any idea if there's an easy way to fix both #4498 and #4565?

@dolfinus
Copy link
Contributor Author

This PR fixes only #4565. It does not fix the #4498, I've never mentioned that

@samuelcolvin
Copy link
Member

I've never mentioned that

I understand that, but the line changed here is the same line discussed in #4498.

Obviously before we release v1.10.3, we need to fix both issues. Hence I'm trying work out a fix that covers both.

Copy link
Member

@PrettyWood PrettyWood left a comment

Choose a reason for hiding this comment

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

I finally made a fix that fixes the related issue + 3 more. I'm sorry @dolfinus but this PR should probably be closed.
I know I took a very long time to make a bug fix but I had way too much on my plate and needed to wait for the holidays to arrive

@dolfinus dolfinus closed this Dec 27, 2022
@dolfinus dolfinus deleted the fix_dataclass_decorator branch December 27, 2022 23:11
@dolfinus
Copy link
Contributor Author

Ok, thanks!

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.

None yet

3 participants