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

cached_property issues #1241

Closed
didimelli opened this issue Feb 18, 2020 · 11 comments · Fixed by #5502
Closed

cached_property issues #1241

didimelli opened this issue Feb 18, 2020 · 11 comments · Fixed by #5502

Comments

@didimelli
Copy link

Unable to use cached_property

Hi,

I am using pydantic for almost any project right now and I find it awesome.

I recently found an handy package, funcy, and I am trying to work with cached_property decorator.

What I am doing is something like this:

class A(BaseModel):

    class Config:
        arbitrary_types_allowed = True

    @cached_property
    def test(self):
        return 10

The arbitrary_types_allowed = True is needed because if not present, a ValidationError is raised:

File "pydantic/main.py", line 215, in pydantic.main.ModelMetaclass.__new__
  File "pydantic/fields.py", line 277, in pydantic.fields.ModelField.infer
  File "pydantic/fields.py", line 249, in pydantic.fields.ModelField.__init__
  File "pydantic/fields.py", line 335, in pydantic.fields.ModelField.prepare
  File "pydantic/fields.py", line 468, in pydantic.fields.ModelField.populate_validators
  File "pydantic/validators.py", line 574, in find_validators
RuntimeError: no validator found for <class 'funcy.objects.cached_property'>, see `arbitrary_types_allowed` in Config

When i do:

a = A()

print(a.test)

# this is what i get
<funcy.objects.cached_property object at 0x7f70466e4748>

# while of course i would like to have 10 as an output.

Same happens with python3.8 cached_property.
If doing the same with a vanilla python class, the behaviour is the expected one (the property outputs 10).

Is there anything I am missing?
Could anyone help me?

Thank you very much, pydantic is really great and you are doing a terrific job.

@samuelcolvin
Copy link
Member

You might be able to use keep_untouched to fix this.

I want to make some improvements to the interface for skipping types, replacing keep_untouched with a more flexible solution, but that's another issue.


I'd be happy to accept a PR to correctly skip python 3.8's cached_property when building fields.

fancy is more complicated since I obviously don't want to make it a dependency of pydantic just to skip fields, I think best to use keep_untouched for this.

@dmontagu
Copy link
Contributor

Yep, just to be clear, it would look like this:

class A(BaseModel):

    class Config:
        arbitrary_types_allowed = True
        keep_untouched = (cached_property,)

    @cached_property
    def test(self):
        return 10

I use this with some of my own models without a problem.

@samuelcolvin samuelcolvin added this to the Version 2 milestone Feb 19, 2020
@didimelli
Copy link
Author

Hi,

thank you very much, it works fine.

The issue now is that you cannot set the property.

e.g.

class A(BaseModel):

    class Config:
        arbitrary_types_allowed = True
        keep_untouched = (cached_property,)

    @cached_property
    def test(self):
        return 10

a = A()
a.test = 15

# Traceback (most recent call last):
#   File "<input>", line 1, in <module>
#   File "pydantic/main.py", line 290, in pydantic.main.BaseModel.__setattr__
# ValueError: "A" object has no field "test"

While with vanilla python classes, the cached_property can be set.

class A:

    @cached_property
    def test(self):
        return 10

a = A()
print(a.test)
# 10
a.test = 15
print(a.test)
# 15

@samuelcolvin
Copy link
Member

that's a separate issue and related to #655.

@dmontagu
Copy link
Contributor

dmontagu commented Mar 3, 2020

Note that, as discussed in #655, you can work around this via object.__setattr__, but also as discussed in #655 the use of @cached_property results in some surprising/confusing behavior related to model serialization:

from functools import cached_property

from pydantic import BaseModel


class A(BaseModel):
    class Config:
        arbitrary_types_allowed = True
        keep_untouched = (cached_property,)

    @cached_property
    def test(self):
        return 10


a = A()
print(repr(a))
# A()
print(a.dict())
# {}

print(a.test)  # causes cached property to be computed and stored on instance
# 10
print(repr(a))
# A(test=10)

# note that the cached property is now part of the unstructuring/serialization,
# even though it wasn't before:
print(a.dict())
# {'test': 10}

object.__setattr__(a, "test", 15)
print(repr(a))
# A(test=15)

@dmontagu
Copy link
Contributor

dmontagu commented Mar 4, 2020

Note that through a custom rework of the cached_property it should be possible to achieve something similar that would never modify the underlying object's __dict__ (which is what is causing the surprising unstructuring/serialization behavior).

Basically, you'd want to keep the cached value living in a property of the custom_cached_property instance, rather than using the instance's __dict__ itself as the cache. This would cause substantially more overhead than the approach used by the functools.cached_property (~4 lookups instead of 1), which is (in part) why it is not used by that implementation, but if the cached property is very expensive to compute this is likely negligible.

And note that to prevent memory leaks you'd probably want the cache to be a weakref.WeakKeyDictionary or similar if you wanted to go down this route of implementation.


Given the complexity here, I'd probably just recommend using a different approach than cached_property if you have any intention of actually serializing/unstructuring the model. (If not, then it's probably fine.)

@samuelcolvin
Copy link
Member

I think the solution is to wait for #935 and either document that cached_property or provide a hard barrier against it's use.

@yajo
Copy link

yajo commented Dec 17, 2020

I tested today and #1241 (comment) works fine even without allow_arbitrary_types=True and with native functools.cached_property using Python 3.9:

>>> from functools import *; import pydantic

>>> class Test():
...     class Config:
...         keep_untouched=(cached_property,)
...     @cached_property
...     def hola(self)->str:
...         print('done')
...         return "yo"

>>> t=Test()

>>> t.hola
done
'yo'


>>> t.hola
'yo'

@ghost
Copy link

ghost commented Aug 10, 2021

from functools import cached_property

from pydantic import BaseModel


class Bar(BaseModel):
    spam: int
    eggs: int

    @cached_property
    def spameggs(self) -> int:
        return self.spam + self.eggs

    class Config:
        keep_untouched = (cached_property,)  # https://github.com/samuelcolvin/pydantic/issues/1241


bar = Bar(spam=10, eggs=20)
print(bar.spameggs)
print(bar)

Run result:

30
Traceback (most recent call last):
  File "example.py", line 20, in <module>
    print(bar)
  File "pydantic/utils.py", line 349, in pydantic.utils.Representation.__str__
  File "pydantic/utils.py", line 331, in genexpr
  File "pydantic/utils.py", line 331, in genexpr
  File "pydantic/main.py", line 941, in pydantic.main.BaseModel.__repr_args__
KeyError: 'spameggs'

Is it related or I need to create a separate issue?

@PrettyWood
Copy link
Member

PrettyWood commented Aug 10, 2021

Hi @AlekseiMarinichenko
It's another issue only on master
I will probably fix it in #2625 when I add support for @computed_field(repr=False)

@dmontagu
Copy link
Contributor

Update: @computed_field is now available in the latest v2 alpha release v2.0a3; some docs are available in https://github.com/pydantic/pydantic/blob/main/docs/usage/computed_fields.md

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

Successfully merging a pull request may close this issue.

5 participants