Skip to content

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

Merged
samuelcolvin merged 7 commits intopydantic:1.10.X-fixesfrom
PrettyWood:fix-dataclasses
Dec 29, 2022
Merged

fix: use dataclasses proxy for frozen or empty dataclasses#4878
samuelcolvin merged 7 commits intopydantic:1.10.X-fixesfrom
PrettyWood:fix-dataclasses

Conversation

@PrettyWood
Copy link
Copy Markdown
Contributor

@PrettyWood PrettyWood commented Dec 26, 2022

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

Comment thread pydantic/dataclasses.py
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
Copy Markdown
Contributor Author

@PrettyWood PrettyWood Dec 26, 2022

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

@PrettyWood
Copy link
Copy Markdown
Contributor Author

Please review

Copy link
Copy Markdown
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.

Comment thread pydantic/dataclasses.py
frozen: bool = False,
config: Union[ConfigDict, Type[object], None] = None,
validate_on_init: Optional[bool] = None,
use_proxy: Optional[bool] = None,
Copy link
Copy Markdown
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.

Comment thread pydantic/dataclasses.py
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
Copy Markdown
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
8 tasks
@PrettyWood
Copy link
Copy Markdown
Contributor 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
Copy link
Copy Markdown
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.

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

@samuelcolvin
Copy link
Copy Markdown
Member

please review

Copy link
Copy Markdown
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
Copy Markdown
Member

thanks so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment