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.

Sign up for GitHub

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

Jump to bottom

fix: use dataclasses proxy for frozen or empty dataclasses #4878

Merged
merged 7 commits into from
Dec 29, 2022
Merged

fix: use dataclasses proxy for frozen or empty dataclasses #4878

merged 7 commits into from
Dec 29, 2022

Conversation

PrettyWood
Copy link
Member

@PrettyWood PrettyWood commented Dec 26, 2022 •
edited
Loading

fix #4498
fix #4565
fix #4877
fix #4585

PrettyWood
PrettyWood commented Dec 26, 2022
View reviewed changes
pydantic/dataclasses.py
@@ -197,7 +189,15 @@ def dataclass(
def wrap(cls: Type[Any]) -> 'DataclassClassOrWrapper':
import dataclasses

if is_builtin_dataclass(cls) and _extra_dc_args(_cls) == _extra_dc_args(_cls.__bases__[0]): # type: ignore
should_use_proxy = (
Copy link
Member Author

@PrettyWood PrettyWood Dec 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I reckon I should have added directly the dataclass kwarg use_proxy and invite people to write
PydanticDC = pydantic.dataclasses.dataclass(DC, use_proxy=True) to keep DC untouched.
It would have been explicit for these rare use cases and would have avoided this kind of (very weak) magic
But I reckon it's too late now for v1 and v2 won't need this at all yay 🎉

Copy link
Member

Choose a reason for hiding this comment

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

you're probably right, but hindsight is a beautiful thing. No worries now.

This was linked to issues Dec 27, 2022
1.10.2 is broken on some frozen dataclasses #4498
Closed
dataclass with methods cannot use init=False fields #4585
Closed
Nested dataclass definitions not working with pydantic #4877
Closed
Null reference error when pydantic dataclass wrapped class inherits from stdlib dataclass without adding new field #4565
Closed
@PrettyWood
Copy link
Member Author

Please review

samuelcolvin
samuelcolvin reviewed Dec 28, 2022
View reviewed changes
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!

This looks great, really pleased you were able to fix it so easily, I wasn't sure where to go.

Just one small suggestion, otherwise this is good to go.

Please update.

pydantic/dataclasses.py
@@ -183,7 +170,7 @@ def dataclass(
unsafe_hash: bool = False,
frozen: bool = False,
config: Union[ConfigDict, Type[object], None] = None,
validate_on_init: Optional[bool] = None,
use_proxy: Optional[bool] = None,
Copy link
Member

Choose a reason for hiding this comment

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

should we allow the validate_on_init and raise a deprecation warning here?

Otherwise this is technically a breaking change.

pydantic/dataclasses.py
@@ -197,7 +189,15 @@ def dataclass(
def wrap(cls: Type[Any]) -> 'DataclassClassOrWrapper':
import dataclasses

if is_builtin_dataclass(cls) and _extra_dc_args(_cls) == _extra_dc_args(_cls.__bases__[0]): # type: ignore
should_use_proxy = (
Copy link
Member

Choose a reason for hiding this comment

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

you're probably right, but hindsight is a beautiful thing. No worries now.

@pydantic-hooky pydantic-hooky bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels Dec 28, 2022
@samuelcolvin samuelcolvin mentioned this pull request Dec 28, 2022
Closed
8 tasks
@PrettyWood
Copy link
Member Author

please review

@pydantic-hooky pydantic-hooky bot added ready for review and removed awaiting author revision awaiting changes from the PR author labels Dec 29, 2022
samuelcolvin
samuelcolvin approved these changes Dec 29, 2022
View reviewed changes
Copy link
Member

@samuelcolvin samuelcolvin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.

Anyone else have any inputs quickly? Otherwise let's merge and get v1.10.3 released.

@samuelcolvin
Copy link
Member

please review

hramezani
hramezani approved these changes Dec 29, 2022
View reviewed changes
Copy link
Member

@hramezani hramezani left a comment

Choose a reason for hiding this comment

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

Great 🎉 Thanks!

@samuelcolvin samuelcolvin merged commit bf4b5ce into pydantic:1.10.X-fixes Dec 29, 2022
@samuelcolvin
Copy link
Member

thanks so much.

This was referenced Dec 29, 2022
Closed
Closed
Closed
Closed
@PrettyWood PrettyWood deleted the fix-dataclasses branch December 29, 2022 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hramezani hramezani hramezani approved these changes

@samuelcolvin samuelcolvin samuelcolvin approved these changes

Assignees

@hramezani hramezani

@samuelcolvin samuelcolvin

@PrettyWood PrettyWood

Labels
ready for review
Projects
None yet
Milestone
No milestone
3 participants
@PrettyWood @samuelcolvin @hramezani