Add private attributes support #1679

Bobronium · 2020-07-02T17:54:10Z

Change Summary

Private attributes declared as regular fields, but always start with underscore and PrivateAttr is used instead of Field. Upon class creation they added in __slots__ and Model.__private_attributes__ for faster lookups and support for default values
Default values declared just like fields: _attr[: annotation] = default_value or _attr = PrivateAttr(default_value)

Example from docs

from datetime import datetime
from random import randint

from pydantic import BaseModel, PrivateAttr


class TimeAwareModel(BaseModel):
    _processed_at: datetime = PrivateAttr(default_factory=datetime.now)
    _secret_value: str = PrivateAttr()

    def __init__(self, **data):
        super().__init__(**data)
        self._secret_value = randint(1, 5)


m = TimeAwareModel()
print(m._processed_at)
print(m._secret_value)

Other changes

Replaced copy.deepcopy() usage with new pydantic.utils.smart_deepcopy() for faster immutable values handling

Related issue number

Closes #655

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
changes/<pull request or issue id>-<github username>.md file added describing change
(see changes/README.md for details)

codecov · 2020-07-02T17:58:24Z

Codecov Report

Merging #1679 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1679   +/-   ##
=======================================
  Coverage   99.90%   99.90%           
=======================================
  Files          21       21           
  Lines        4035     4051   +16     
  Branches      805      807    +2     
=======================================
+ Hits         4031     4047   +16     
  Misses          3        3           
  Partials        1        1

Impacted Files	Coverage Δ
pydantic/class_validators.py	`100.00% <100.00%> (ø)`
pydantic/fields.py	`100.00% <100.00%> (ø)`
pydantic/schema.py	`100.00% <100.00%> (ø)`
pydantic/utils.py	`100.00% <100.00%> (ø)`
pydantic/generics.py	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bfee87...104d600. Read the comment docs.

Bobronium · 2020-07-03T20:56:52Z

@samuelcolvin, seems like test_constrained_set_default introduced in dca9855 was broken somehow. #1627 (comment)

Benoss · 2020-08-24T07:32:17Z

I quite like this way of doing it

samuelcolvin

sorry it's been so long, I've been really busy.

This looks amazing, but might need to be split into multiple PRs.

docs/usage/models.md

samuelcolvin · 2020-09-06T17:47:03Z

pydantic/utils.py

+    bool,
+    bytes,
+    type,
+    type(None),


Suggested change

type(None),

NoneType,

(imported from ./typing.py)

samuelcolvin · 2020-09-06T17:47:31Z

pydantic/utils.py

 )

+ROOT_KEY = '__root__'
+IMMUTABLE_NON_COLLECTIONS_TYPES: AbstractSet[Type[Any]] = {


Suggested change

IMMUTABLE_NON_COLLECTIONS_TYPES: AbstractSet[Type[Any]] = {

IMMUTABLE_NON_COLLECTIONS_TYPES: Set[Type[Any]] = {

Any reason not to use normal Set here?

might be worth adding a comment explaining this? Also maybe IMMUTABLE_TYPES would be a better name?

might be worth adding a comment explaining this?

Sure, I commented usage of this constant. Should I describe it alone here?

Also maybe IMMUTABLE_TYPES would be a better name?

Well, tuple() is immutable type and yet could not be included in this set, because it can contain mutable types, so I thought it's worth clarifying

samuelcolvin · 2020-09-06T17:48:46Z

pydantic/utils.py

@@ -94,7 +112,7 @@ def validate_field_name(bases: List[Type['BaseModel']], field_name: str) -> None
    Ensure that the field's name does not shadow an existing attribute of the model.
    """
    for base in bases:
-        if getattr(base, field_name, None):
+        if hasattr(base, field_name):


please remove this change, it's unrelated.

samuelcolvin · 2020-09-06T17:50:34Z

pydantic/utils.py

+Obj = TypeVar('Obj')
+
+
+def smart_deepcopy(obj: Obj) -> Obj:


this looks very cool, but are we 100% sure it always works? Or just works in all the cases we know of?

Also, this is great, but might be best to implement it as a separate PR, it's not really related to the core change here.

You're right, separate PR is more sutable for this change.

Or just works in all the cases we know of?

Good catch. I think implementing faster behaviour only for built-in collections and immutable types is a way to go.

samuelcolvin · 2020-09-06T17:55:34Z

pydantic/main.py

+        private_attributes: Dict[str, Any] = dict.fromkeys((slots,) if isinstance(slots, str) else slots, Undefined)
+        validate_private_attributes(private_attributes)
+
+        for base in reversed(bases) if _is_base_model_class_defined else ():


is this just a cosmetic change? if so could you remove it to make this change easier to review, if not I think it belongs in another PR.

Oof, unfortunately I don't remember. Should've put a comment here 🤦 . But from what I see, now it includes BaseModel and it didn't before. I'll try to remember the reason I did it this way and either put explaning comment, or change the line back.

samuelcolvin · 2020-09-06T17:57:10Z

pydantic/main.py

@@ -303,7 +306,8 @@ def __new__(mcs, name, bases, namespace, **kwargs):  # noqa C901
            '__schema_cache__': {},
            '__json_encoder__': staticmethod(json_encoder),
            '__custom_root_type__': _custom_root_type,
-            **{n: v for n, v in namespace.items() if n not in fields},
+            '__private_attributes__': private_attributes,
+            **{n: v for n, v in namespace.items() if n not in fields | private_attributes.keys()},


humm, does this create fields | private_attributes.keys() on every iteration?

Either way, might be clearer to define it beforehand.

samuelcolvin · 2020-09-06T18:00:53Z

pydantic/main.py

+        object_setattr(self, '__fields_set__', state['__fields_set__'])
+        self._set_private_attributes(state['__private_attributes_values__'], need_copy=False, check=False)
+
+    def _set_private_attributes(self, source: 'DictStrAny', need_copy: bool = True, check: bool = True) -> None:


Looks like the two kwargs are always used together, maybe they can be combined into check_copy or from_pickle=False?

This method might need a docstring too.

samuelcolvin · 2020-09-06T18:01:51Z

pydantic/main.py

+            if value is not Undefined:
+                object_setattr(self, name, smart_deepcopy(value) if need_copy else value)
+            elif check and not hasattr(self, name):
+                raise AttributeError(f'private attribute "{name}" is unset')


maybe if check is True we should do the check first so the exception message is always the same?

or make it identical here.

Didn't get this one, could you explain?

samuelcolvin · 2020-09-06T18:03:39Z

docs/examples/private_attributes.py

+    return max(storage) + 1 if storage else 0
+
+
+class BaseStorageModel(BaseModel):


maybe the _processed_at example at the top of #655 is simpler and clearer?

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

Bobronium

@samuelcolvin, thanks for review, I answered some comments and think I'll be able to split PR on the next weekend.

Bobronium · 2020-09-06T18:54:04Z

pydantic/utils.py

 )

+ROOT_KEY = '__root__'
+IMMUTABLE_NON_COLLECTIONS_TYPES: AbstractSet[Type[Any]] = {


might be worth adding a comment explaining this?

Sure, I commented usage of this constant. Should I describe it alone here?

Also maybe IMMUTABLE_TYPES would be a better name?

Well, tuple() is immutable type and yet could not be included in this set, because it can contain mutable types, so I thought it's worth clarifying

Bobronium · 2020-09-06T19:02:01Z

pydantic/utils.py

+Obj = TypeVar('Obj')
+
+
+def smart_deepcopy(obj: Obj) -> Obj:


You're right, separate PR is more sutable for this change.

Or just works in all the cases we know of?

Good catch. I think implementing faster behaviour only for built-in collections and immutable types is a way to go.

Bobronium · 2020-09-06T19:10:26Z

pydantic/main.py

-        object.__setattr__(self, '__fields_set__', state['__fields_set__'])
+        object_setattr(self, '__dict__', state['__dict__'])
+        object_setattr(self, '__fields_set__', state['__fields_set__'])
+        self._set_private_attributes(state['__private_attributes_values__'], need_copy=False, check=False)


@samuelcolvin, maybe we should use dict.get() here, so unpickling models dumped with older versions of pydantic would work?

Suggested change

self._set_private_attributes(state['__private_attributes_values__'], need_copy=False, check=False)

self._set_private_attributes(state.get('__private_attributes_values__', {}), need_copy=False, check=False)

Yes makes sense.

Bobronium · 2020-09-06T19:15:27Z

pydantic/main.py

+        private_attributes: Dict[str, Any] = dict.fromkeys((slots,) if isinstance(slots, str) else slots, Undefined)
+        validate_private_attributes(private_attributes)
+
+        for base in reversed(bases) if _is_base_model_class_defined else ():


Oof, unfortunately I don't remember. Should've put a comment here 🤦 . But from what I see, now it includes BaseModel and it didn't before. I'll try to remember the reason I did it this way and either put explaning comment, or change the line back.

* add smart_deepcopy * uncomment tuple in BUILTIN_COLLECTIONS, fix doc a bit * Fix grammar Co-authored-by: PrettyWood <em.jolibois@gmail.com> * replace map() usage with generator comprehension, fix comment Co-authored-by: PrettyWood <em.jolibois@gmail.com>

samuelcolvin

Most things look good here, met me know if I've missed any questions.

samuelcolvin · 2020-10-18T19:08:00Z

pydantic/main.py

-        object.__setattr__(self, '__fields_set__', state['__fields_set__'])
+        object_setattr(self, '__dict__', state['__dict__'])
+        object_setattr(self, '__fields_set__', state['__fields_set__'])
+        self._set_private_attributes(state['__private_attributes_values__'], need_copy=False, check=False)


Yes makes sense.

samuelcolvin · 2020-10-18T19:08:55Z

pydantic/utils.py

 )

+ROOT_KEY = '__root__'
+IMMUTABLE_NON_COLLECTIONS_TYPES: AbstractSet[Type[Any]] = {


Bobronium · 2020-10-21T13:33:22Z

I was procrastinating about this PR recently, as I thought that it might be worth adding support of default_factory for private fields, as long as other fields features maybe. But don't see any obvious way to do it. Maybe we should think it throw better before sticking with final implementation?

For example, this will give error about _processed_at was not set, because code where it's set is executed after BaseModel.__init__

from pydantic import BaseModel
from datetime import datetime

class TestExtra(BaseModel):
    a: int
    __slots__ = ('_processed_at',)
    _processed_at: datetime

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self._processed_at = datetime.utcnow()

TestExtra(**{"a": 1})

I would prefer having something like this: (__slots__ are automatically constructed in ModelMetaclass.__new__, value is generated in BaseModel.__init__, or in validate_model)

from pydantic import BaseModel, PrivateField
from datetime import datetime

class TestExtra(BaseModel):
    a: int
    _processed_at: datetime = PrivateField(default_factory=datetime)  # or Field(private=True, ...

TestExtra(**{"a": 1})

samuelcolvin · 2020-10-25T14:17:24Z

automatically generating __slots__ would be great, I think this would make it much easier to use.

Bobronium · 2020-10-25T14:21:52Z

What do you think about PrivateField? Should it subclass regular field class, be an independent class, or it's better to use Field(private=True)?

# Conflicts: # pydantic/main.py # pydantic/utils.py # tests/test_utils.py

samuelcolvin · 2020-10-25T15:05:24Z

I think probably easiest if it's a regular class.

I think we should keep it separate from Field because the point is that these attributes are NOT fields.

# Conflicts: # pydantic/main.py # pydantic/schema.py # tests/test_construction.py

(see comment in PR)

Bobronium · 2020-10-25T18:49:17Z

pydantic/main.py


    @no_type_check
-    def __setattr__(self, name, value):
+    def __setattr__(self, name, value):  # noqa: C901 (ignore complexity)


had to set # noqa: C901 (ignore complexity) here because of today's changes in #1972. Is this the way to go, or something should be moved outside of this method?

i think this is fine.

samuelcolvin · 2020-10-25T19:46:11Z

@MrMrRobat this is looking good, do you think it will be ready to merge today?

I want to get v1.7 released, but I'm happy wait until tomorrow if we can include this?

Bobronium · 2020-10-25T19:51:32Z

do you think it will be ready to merge today?

Yeah, if don't have any comments or suggestions, it's ready :)

samuelcolvin

looking good, just a few things to clarify.

samuelcolvin · 2020-10-25T20:03:16Z

pydantic/main.py

-        object.__setattr__(__pydantic_self__, '__fields_set__', fields_set)
+        object_setattr(__pydantic_self__, '__dict__', values)
+        object_setattr(__pydantic_self__, '__fields_set__', fields_set)
+        __pydantic_self__._set_default_private_attributes(__pydantic_self__.__private_attributes__)


why is this required?

what do you mean exactly?

Can we avoid this line since this is the critical path in terms of performance?

It sets defaults of __private_attributes__, without this line default not default_factoru wouldn't work.
What's critical here in terms of performance? Function call itself or executing code of the function?

Ok, I guess we need it.

Initialising/validating models is the critical path, I don't want to slow that down unless we absolutely have to.

samuelcolvin · 2020-10-25T20:05:25Z

docs/examples/private_attributes_underscore_attrs_are_private.py

+    __private_attr__: str = 'private attr value'
+
+    class Config:
+        underscore_attrs_are_private = True


I this needs documenting in model_config.md. Also we need to be clear exactly what form names need to be in.

It looks from this like if you use PrivateAttr you can use _whatever, but with underscore_attrs_are_private you have to use __whatever__, but reading the code, I think that's not the case.

actually you've written it clear in the copy, but still better to use the same format in both code examples.

Is there anything special about __whatever__ or is it just that it starts with an underscore, like _whatever?

Since I think according to some PEP we're not supposed to use dunder methods, unless they're official to python, maybe leave them out of the example and just mention them in the text?

docs/examples/private_attributes.py

pydantic/main.py

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

samuelcolvin

I think this is looking amazing, thank you @MrMrRobat.

I've tried it myself and i think it'll be extremely useful.

I have one small proposed change in name, but only other question is on model.copy() which currently doesn't copy private attributes, but instead initialises from the default or default factory. I think I would expect copy() to (deep?)copy the private attributes, a bit like __setstate__.

What do you think?

pydantic/main.py

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

samuelcolvin · 2020-10-26T09:11:49Z

you need to reference the new name, sorry I should have made that clear in my change proposal.

Will you change it or will I?

Bobronium · 2020-10-26T09:23:04Z

I think I would expect copy() to (deep?)copy the private attributes, a bit like setstate.

Yeah, you're right. Working on it.

Bobronium · 2020-10-26T09:36:53Z

Working on it.

Done

samuelcolvin · 2020-10-26T09:48:03Z

awesome, thank you.

I'm really looking forward to using this.

Thanks `PrivateAttr` and pydantic/pydantic#1679.

samuelcolvin · 2022-09-14T16:37:17Z

Question on this in #4518.

Bobronium added 2 commits July 2, 2020 20:45

Add private attributes support

5964240

Add more blank lines in example

d783caf

Bobronium and others added 2 commits July 2, 2020 21:13

Add changes file

d4dabc4

Merge branch 'master' into add-private-attributes

c182678

PrettyWood mentioned this pull request Jul 23, 2020

Class attributes starting with underscore do not support assignment #288

Closed

PrettyWood mentioned this pull request Aug 24, 2020

Is it possible to exclude fields from the model itself when exporting them? (instead of dict(exclude=..)) #1862

Closed

samuelcolvin reviewed Sep 6, 2020

View reviewed changes

Update docs/usage/models.md

1c7256a

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

Bobronium commented Sep 6, 2020

View reviewed changes

samuelcolvin mentioned this pull request Oct 8, 2020

add smart_deepcopy (originaly from #1679) #1920

Merged

4 tasks

samuelcolvin reviewed Oct 18, 2020

View reviewed changes

Merge branch 'master' into add-private-attributes

ef640ff

# Conflicts: # pydantic/main.py # pydantic/utils.py # tests/test_utils.py

Bobronium added 5 commits October 25, 2020 18:05

fix after bad merge

3868ad3

Add PrivateAttr, Config.underscore_attrs_are_private

d60de4c

remove unrelated change in utils.py

9096f63

Merge branch 'master' into add-private-attributes

90576d2

# Conflicts: # pydantic/main.py # pydantic/schema.py # tests/test_construction.py

add # noqa: C901 (ignore complexity) to __setattr__

a701b3e

(see comment in PR)

Bobronium commented Oct 25, 2020

View reviewed changes

samuelcolvin reviewed Oct 25, 2020

View reviewed changes

add annotation to Config.underscore_attrs_are_private

33e3f28

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

Bobronium and others added 7 commits October 25, 2020 23:18

use sunder names

dcfee02

mention underscore_attrs_are_private in model_config.md

063ba48

add comment about default factory

1333ffb

fix comment

788d24b

fix comment

dedb465

clarify that both dunder and sunder names might be used

6ea1a1f

tweak docs and name

08d6f5d

samuelcolvin reviewed Oct 26, 2020

View reviewed changes

pydantic/main.py Outdated Show resolved Hide resolved

_set_default_private_attributes -> _init_private_attributes

60ebda2

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

samuelcolvin and others added 2 commits October 26, 2020 09:15

use new name _init_private_attributes

2689d36

move tests

afa3fb9

Bobronium added 3 commits October 26, 2020 12:28

copy private attributes in BaseModel.copy()

47464ba

add test for default and default_factory used together

7fe179a

fix linting

2090ba4

more tests, default_factory kw only

104d600

samuelcolvin merged commit 664cbcf into pydantic:master Oct 26, 2020

PrettyWood mentioned this pull request Nov 23, 2020

fix: support underscore_attrs_are_private with generic models #2139

Merged

4 tasks

PrettyWood mentioned this pull request Jan 24, 2021

Question: add private attribute #655

Closed

luckydonald added a commit to luckydonald/fastorm that referenced this pull request Oct 27, 2021

Fixed the database cache properly.

71c5f64

Thanks `PrivateAttr` and pydantic/pydantic#1679.

	IMMUTABLE_NON_COLLECTIONS_TYPES: AbstractSet[Type[Any]] = {
	IMMUTABLE_NON_COLLECTIONS_TYPES: Set[Type[Any]] = {

		return max(storage) + 1 if storage else 0


		class BaseStorageModel(BaseModel):

	self._set_private_attributes(state['__private_attributes_values__'], need_copy=False, check=False)
	self._set_private_attributes(state.get('__private_attributes_values__', {}), need_copy=False, check=False)

Add private attributes support #1679

Add private attributes support #1679

Conversation

Bobronium commented Jul 2, 2020 • edited

Change Summary

Example from docs

Other changes

Related issue number

Checklist

codecov bot commented Jul 2, 2020 • edited

Codecov Report

Bobronium commented Jul 3, 2020 • edited

Benoss commented Aug 24, 2020

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobronium Sep 6, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobronium left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobronium Sep 6, 2020 • edited

Choose a reason for hiding this comment

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobronium commented Oct 21, 2020 • edited

samuelcolvin commented Oct 25, 2020

Bobronium commented Oct 25, 2020

samuelcolvin commented Oct 25, 2020

Bobronium Oct 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelcolvin commented Oct 25, 2020

Bobronium commented Oct 25, 2020

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin commented Oct 26, 2020

Bobronium commented Oct 26, 2020 • edited

Bobronium commented Oct 26, 2020

samuelcolvin commented Oct 26, 2020

samuelcolvin commented Sep 14, 2022

Bobronium commented Jul 2, 2020 •

edited

codecov bot commented Jul 2, 2020 •

edited

Bobronium commented Jul 3, 2020 •

edited

Bobronium Sep 6, 2020 •

edited

Bobronium Sep 6, 2020 •

edited

Bobronium commented Oct 21, 2020 •

edited

Bobronium Oct 25, 2020 •

edited

Bobronium commented Oct 26, 2020 •

edited