Repository navigation
MNT: Replace lazyproperty with cached_property in Python 3.8 #9036
Description
Activity
what about renaming our
lazyproperty, so it'll be easier to switch it out once the time is here to require 3.8+?Existing package you might want to look into if you're not using Python 3.8 yet: https://pypi.org/project/cached-property/
Reacted by P. L. Lim@bsipocz , it is a possibility, though it would be more like alias than renaming, because we need to keep
lazypropertyaround for a while yet for backward compatibility. However, we do need to double check that they are indeed completely equivalent before making that alias.Indeed, first check equivalence, but then we could follow a dual track of, under python >=3.8 doing effectively
from functools import cached_property as lazypropertyand then deprecatinglazypropertyonce our minimum version is 3.8. Not that different than what we did withOrderedDict, etc.Hmm, from a quick look,
cached_propertydoes not have the ability to have asetterordeleter- I definitely use the latter in my code. But perhaps we can upstream our implementations for those.Reacted by Mariatta, Thomas Robitaille, Hannes Breytenbach and Simon ConseilReacted by Brigitta Sipőcz and Thomas RobitailleI don't think we should rename ours - I think we should just keep and then eventually deprecate/remove it once
cached_propertycan be used instead. If we rename, we have to deprecate the old name, then deprecate the removal separately.Reacted by P. L. LimI'm generally in favor of this though ditto @mhvk that cached_property does not support custom setters and deleters, and lazyproperty's mechanism for setters is somewhat weird, but also useful, and is used in astropy.io.fits. However, it might be possible to re-implement most of
lazypropertyas a subclass ofcached_propertywith support for setters and deleters. I haven't tried it yet though.Another point about both
lazypropertyandcached_propertyis they both require types that have a__dict__, which is a limitation I have in fact run up against in some cases. In one of my other projects I implemented something similar to this but a little more sophisticated in that it can have a custom caching mechanism. The default is to use__dict__but you can subclass it to provide a different mechanism for storing/retrieving cached values. This makes it usable with arbitrary types that don't have a__dict__, or even using external caches allowing already computed values to be reused across runs if applicable.Looking again, I think
cached_propertyis really a bit of a different beast. As it does not have__set__and__delete__, it is not a data descriptor: the moment it sets its own name inparent.__dict__, any further access will retrieve that information -- the descriptor is now inactive (until one deletes the attribute). It also means one can just set the attribute at will. Really it is more like an attribute with a default value, while inlazypropertyone always goes through the descriptor to retrieve/set/delete the attribute.So, my sense is that when 3.8 is our minimum it will be good to have a more detailed look but I'd guess that only in some places
cached_propertyis a good replacement forlazyproperty. What would probably be good is to update the docstring forlazypropertyto refer tocached_propertyand describe the differences.I'm not sure what you mean by "the descriptor is now inactive". It works pretty much the same way lazyproperty does in that it stores the cached value in each instance's
__dict__. The descriptor still remains in the class__dict__and lookups of that attribute on the instance still go through the descriptor. The only difference is the lack of setter and deleter support. The rest is almost identical (esp. since #11221) which is a good point in its favor.@embray - if I understand https://docs.python.org/3/reference/datamodel.html#invoking-descriptors properly, for
cached_property, the moment it has stored the value in the instance__dict__, lookup will directly use that value when you look up the attribute, not go throughcached_property.__get__again, becausecached_propertyhas only defined__get__, not__set__, and thus is not a data descriptor.I see, so it is a non-data descriptor (which confused me since just
propertyis a data-descriptor). It's confusing terminology because it still returns "data" sigh.Yes, I think it is quite confusing to call it
cached_property(and I certainly was confused when I looked at the code first), as its setting and deleting have very different behaviour from that of aproperty. It is more like "an attribute with lazily evaluated default".Indeed, though as discussed above, we have to be really careful, since the behaviour is rather different. Definitely not a search-and-replace!
Reacted by Simon ConseilRe-reading the discussions about concerns of
cached_propertyis not a true "data descriptor," perhaps this will not be resolved all in one PR but rather have to be done in parts, one PR per sub-package, so the sub-package maintainers could each evaluate whether substitution is feasible or not. But it is looking like we can never truly get rid of ourlazypropertyhere.Currently, I see it used in the following core subpackages. I am sure it is also used downstream but I don't know where.
constantscoordinatescosmologyio.fitsnddatatimeunitswcs
I think once we have decided on which ones can be replaced and which cannot, we should open smaller issues that is actually actionable and close this one out.
This came up over this year's Coordination Meeting within the free-threading/multithreading breakout session. @larrybradley mentioned that
@lazypropertywasn't multi-threading friendly, so finishing this issue is a sub-goal towards free-threading compat.Reacted by Larry Bradley@lazypropertyholds a singlethreading.RLockper property definition, shared by every instance of the class. In a free-threaded interpreter where many threads each work on their own object, those threads queue on the same lock and the expensive computation runs one at a time, defeating the parallelism.functools.cached_propertyremoved a similar lock in Python 3.12 (python/cpython#101890) to allow parallel computations. I'm in favor of migrating tocached_property.
As @Mariatta pointed out in #8881 (comment) , we might be able to replace
lazypropertywith cached_property that is new in Python 3.8. We also need to figure out the best way to support this for Python<=3.7 or wait until our Python minversion is 3.8.