Navigation Menu

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: dataclass wrapper was not always called #4484

Merged
merged 1 commit into from Sep 5, 2022

Conversation

PrettyWood
Copy link
Member

Change Summary

In a stdlib dataclass without __post_init__, __post_init__ call is not added in the generated __init__.
So when we inherit from a stdlib dataclass and we add a __post_init__, it won't be called. So all its related code + validation will be ignored.
Now it should be okish (not ok because it's a mess).

In v2, we should rely on stdlib dataclasses and go with a more functional approach

Related issue number

fix #4477

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details.
    You can skip this check if the change does not need a change file.)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

LGTM, I'll fix the markdown in the change file when building history (no need for extra builds).

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM, please update.

changes/4477-PrettyWood.md Outdated Show resolved Hide resolved
pydantic/dataclasses.py Outdated Show resolved Hide resolved
@PrettyWood
Copy link
Member Author

PrettyWood commented Sep 5, 2022

The fix is wrong. I'm pushing something new that handles properly inheritance and stuff (I added a new test).
But it's not acceptable yet. The check on is_builtin_dataclass needs to be more resilient (it's a mess honestly...)

@PrettyWood PrettyWood changed the title fix: always call __post_init__ when set in dataclasses fix: dataclass wrapper was not always called Sep 5, 2022
@samuelcolvin
Copy link
Member

😞 I agree, let me know when it's ready to review.

@PrettyWood PrettyWood force-pushed the fix-dc-post-init branch 2 times, most recently from e489ea9 to d5fe6f8 Compare September 5, 2022 15:35
@PrettyWood PrettyWood marked this pull request as ready for review September 5, 2022 15:36
@samuelcolvin
Copy link
Member

@PrettyWood what do you think, maybe better to wait for another patch release and get the other changes released?

@PrettyWood
Copy link
Member Author

@samuelcolvin it's good! Sorry I had client calls all day. Hard to do that at the same time 😓

@samuelcolvin
Copy link
Member

@samuelcolvin it's good! Sorry I had client calls all day. Hard to do that at the same time sweat

I'm sure! Sorry about this.

@samuelcolvin samuelcolvin merged commit f1e9883 into pydantic:main Sep 5, 2022
@samuelcolvin
Copy link
Member

LGTM, thanks so much.

@samuelcolvin samuelcolvin mentioned this pull request Sep 5, 2022
6 tasks
@PrettyWood PrettyWood deleted the fix-dc-post-init branch September 5, 2022 16:51
@PrettyWood
Copy link
Member Author

PrettyWood commented Sep 5, 2022

I feel like __pydantic_model__ and all should be attached to the dataclass wrapper and not the dataclass itself. But I'm too tired to think about it now. Hopefully we don't have new bugs with dataclasses in v1 and we'll rely directly on stdlib in v2

@samuelcolvin
Copy link
Member

agreed, the whole idea of the Argument type (pydantic/pydantic-core#203) is that we can just call

args, kwargs = schema_validator.validate_python(input)
return dc(*args, **kwargs)

and never touch the dataclass.

@PrettyWood
Copy link
Member Author

Yup! That's great 👍

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.

__post_init__ and validators are not triggered in class inherited from stdlib dataclass in 1.10.1
2 participants