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: Allow inheriting dataclasses with defaults #7184

Merged
merged 4 commits into from Aug 25, 2023
Merged

fix: Allow inheriting dataclasses with defaults #7184

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1fed052
fix: Allow inheriting dataclasses with defaults
tobni Aug 20, 2023
8056cc6
test: Add failing test for stdlib default factory error
tobni Aug 25, 2023
fb35206
test: Add explanatory docstring and showcase supported use
tobni Aug 25, 2023
a1b87f2
test: use pytest.raises
tobni Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions pydantic/_internal/_fields.py
View file
Open in desktop
Expand Up @@ -147,10 +147,18 @@ def collect_model_fields( # noqa: C901
# "... shadows an attribute" errors
generic_origin = getattr(cls, '__pydantic_generic_metadata__', {}).get('origin')
for base in bases:
dataclass_fields = {
field.name for field in (dataclasses.fields(base) if dataclasses.is_dataclass(base) else ())
}
if hasattr(base, ann_name):
if base is generic_origin:
# Don't error when "shadowing" of attributes in parametrized generics
continue

if ann_name in dataclass_fields:
# Don't error when inheriting stdlib dataclasses whose fields are "shadowed" by defaults being set
# on the class instance.
continue
warnings.warn(
f'Field name "{ann_name}" shadows an attribute in parent "{base.__qualname__}"; ',
UserWarning,
Expand Down
47 changes: 47 additions & 0 deletions tests/test_dataclasses.py
View file
Open in desktop
Expand Up @@ -2500,3 +2500,50 @@ class StdLibDataclass:

assert is_pydantic_dataclass(PydanticDataclass) is True
assert is_pydantic_dataclass(StdLibDataclass) is False


def test_can_inherit_stdlib_dataclasses_with_defaults():
@dataclasses.dataclass
class Base:
a: None = None

class Model(BaseModel, Base):
pass

assert Model().a is None


@pytest.mark.xfail(raises=ValidationError, require=True)
def test_can_inherit_stdlib_dataclasses_default_factories_and_use_them():
"""This test documents that default factories are not supported"""

@dataclasses.dataclass
class Base:
a: str = dataclasses.field(default_factory=lambda: 'TEST')

class Model(BaseModel, Base):
pass

assert Model().a == 'TEST'
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather see with pytest.raises(...) since that's what we use elsewhere



def test_can_inherit_stdlib_dataclasses_default_factories_and_provide_a_value():
@dataclasses.dataclass
class Base:
a: str = dataclasses.field(default_factory=lambda: 'TEST')

class Model(BaseModel, Base):
pass

assert Model(a='NOT_THE_SAME').a == 'NOT_THE_SAME'


def test_can_inherit_stdlib_dataclasses_with_dataclass_fields():
@dataclasses.dataclass
class Base:
a: int = dataclasses.field(default=5)

class Model(BaseModel, Base):
pass

assert Model().a == 5
Comment on lines +2541 to +2549
Copy link
Member

Choose a reason for hiding this comment

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

Can we also add a test for default_factory? I'm assuming it doesn't work?

Copy link
Contributor Author

@tobni tobni Aug 25, 2023 •
edited

Choose a reason for hiding this comment

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

@adriangb I added a test that xfails. The internals of stdlib dataclasses does not assign default factory attributes to the class instance (meaning the warning isnt triggered in this case). It also means that you are right, the default factory is not propagated to the resulting FieldInfo instance (because the dataclass.Field isnt assigned to Base but rather Base.__dataclass_fields__.

@dataclasses.dataclass
class Base:
    a: int = dataclasses.field(default_factory=lambda: 5)

assert hasattr(Base, 'a')  # this fails

Copy link
Contributor Author

@tobni tobni Aug 25, 2023 •
edited

Choose a reason for hiding this comment

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

The inheritance works fine if you provide a value, so for my usecase this isn't an important feature to support. I added a test for what we need to work, though.