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: use dataclasses proxy for frozen or empty dataclasses #4878

Merged
merged 7 commits into from Dec 29, 2022

Conversation

PrettyWood
Copy link
Member

@PrettyWood PrettyWood commented Dec 26, 2022

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

@@ -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

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.

@PrettyWood
Copy link
Member Author

Please review

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.

@@ -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.

@@ -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.

@PrettyWood
Copy link
Member Author

please review

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.

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

@samuelcolvin
Copy link
Member

please review

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.

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