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

Fixing __iter__ returning private cached_property info #7570

Merged
merged 5 commits into from Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 53 additions & 3 deletions pydantic/fields.py
Expand Up @@ -7,6 +7,12 @@
import typing
from copy import copy
from dataclasses import Field as DataclassField

try:
from functools import cached_property # type: ignore
except ImportError:
# python 3.7
cached_property = None
from typing import Any, ClassVar
from warnings import warn

Expand Down Expand Up @@ -991,14 +997,26 @@ def computed_field(__func: PropertyT) -> PropertyT:
...


def _wrapped_property_is_private(property_: cached_property | property) -> bool: # type: ignore
"""Returns true if provided property is private, False otherwise."""
wrapped_name: str = ''

if isinstance(property_, property):
wrapped_name = getattr(property_.fget, '__name__', '')
elif isinstance(property_, cached_property): # type: ignore
wrapped_name = getattr(property_.func, '__name__', '') # type: ignore

return wrapped_name.startswith('_') and not wrapped_name.startswith('__')


def computed_field(
__f: PropertyT | None = None,
*,
alias: str | None = None,
alias_priority: int | None = None,
title: str | None = None,
description: str | None = None,
repr: bool = True,
repr: bool | None = None,
sydney-runkle marked this conversation as resolved.
Show resolved Hide resolved
return_type: Any = PydanticUndefined,
) -> PropertyT | typing.Callable[[PropertyT], PropertyT]:
"""Decorator to include `property` and `cached_property` when serializing models or dataclasses.
Expand Down Expand Up @@ -1092,14 +1110,40 @@ def a(self) -> str:
#> ValueError("you can't override a field with a computed field")
```

Private properties decorated with `@computed_field` have `repr=False` by default.

```py
from functools import cached_property

from pydantic import BaseModel, computed_field

class Model(BaseModel):
foo: int

@computed_field
@cached_property
def _private_cached_property(self) -> int:
return -self.foo

@computed_field
@property
def _private_property(self) -> int:
return -self.foo

m = Model(foo=1)
print(repr(m))
#> M(foo=1)
```

Args:
__f: the function to wrap.
alias: alias to use when serializing this computed field, only used when `by_alias=True`
alias_priority: priority of the alias. This affects whether an alias generator is used
title: Title to used when including this computed field in JSON Schema, currently unused waiting for #4697
description: Description to used when including this computed field in JSON Schema, defaults to the functions
docstring, currently unused waiting for #4697
repr: whether to include this computed field in model repr
repr: whether to include this computed field in model repr.
Default is `False` for private properties and `True` for public properties.
return_type: optional return for serialization logic to expect when serializing to JSON, if included
this must be correct, otherwise a `TypeError` is raised.
If you don't include a return type Any is used, which does runtime introspection to handle arbitrary
Expand All @@ -1118,7 +1162,13 @@ def dec(f: Any) -> Any:
# if the function isn't already decorated with `@property` (or another descriptor), then we wrap it now
f = _decorators.ensure_property(f)
alias_priority = (alias_priority or 2) if alias is not None else None
dec_info = ComputedFieldInfo(f, return_type, alias, alias_priority, title, description, repr)

if repr is None:
repr_: bool = False if _wrapped_property_is_private(property_=f) else True
else:
repr_ = repr

dec_info = ComputedFieldInfo(f, return_type, alias, alias_priority, title, description, repr_)
return _decorators.PydanticDescriptorProxy(f, dec_info)

if __f is None:
Expand Down
2 changes: 1 addition & 1 deletion pydantic/main.py
Expand Up @@ -887,7 +887,7 @@ class MyModel(BaseModel, extra='allow'):

def __iter__(self) -> TupleGenerator:
"""So `dict(model)` works."""
yield from self.__dict__.items()
yield from [(k, v) for (k, v) in self.__dict__.items() if not k.startswith('_')]
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it should be it's own PR. The repr thing is one thing, the iter change another

extra = self.__pydantic_extra__
if extra:
yield from extra.items()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_computed_fields.py
Expand Up @@ -402,7 +402,7 @@ def test_private_computed_field():
class MyModel(BaseModel):
x: int

@computed_field
@computed_field(repr=True)
def _double(self) -> int:
return self.x * 2

Expand Down
58 changes: 57 additions & 1 deletion tests/test_private_attributes.py
Expand Up @@ -4,7 +4,7 @@
import pytest
from pydantic_core import PydanticUndefined

from pydantic import BaseModel, ConfigDict, PrivateAttr
from pydantic import BaseModel, ConfigDict, PrivateAttr, computed_field


def test_private_attribute():
Expand Down Expand Up @@ -393,3 +393,59 @@ class BaseConfig(BaseModel):
)

assert module.BaseConfig._FIELD_UPDATE_STRATEGY == {}


@pytest.mark.skipif(not hasattr(functools, 'cached_property'), reason='cached_property is not available')
def test_private_properties_not_included_in_iter_cached_property() -> None:
class Model(BaseModel):
foo: int

@computed_field
@functools.cached_property
def _foo(self) -> int:
return -self.foo

m = Model(foo=1)
assert '_foo' not in list(k for k, _ in m)


def test_private_properties_not_included_in_iter_property() -> None:
class Model(BaseModel):
foo: int

@computed_field
@property
def _foo(self) -> int:
return -self.foo

m = Model(foo=1)
assert '_foo' not in list(k for k, _ in m)


def test_private_properties_not_included_in_repr_by_default_property() -> None:
class Model(BaseModel):
foo: int

@computed_field
@property
def _private_property(self) -> int:
return -self.foo

m = Model(foo=1)
m_repr = repr(m)
assert '_private_property' not in m_repr


@pytest.mark.skipif(not hasattr(functools, 'cached_property'), reason='cached_property is not available')
def test_private_properties_not_included_in_repr_by_default_cached_property() -> None:
class Model(BaseModel):
foo: int

@computed_field
@functools.cached_property
def _private_cached_property(self) -> int:
return -self.foo

m = Model(foo=1)
m_repr = repr(m)
assert '_private_cached_property' not in m_repr