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

Use functools.cached_property on 3.8+ and for TYPE_CHECKING #1417

Merged
merged 8 commits into from
Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ Release date: TBA
* Add new (optional) ``doc_node`` attribute to ``nodes.Module``, ``nodes.ClassDef``,
and ``nodes.FunctionDef``.

* On Python 3.8+ we now use ``functools.cached_property`` instead of our own implementation.
At the same time, ``cachedproperty`` has been deprecated for Python 3.8+.

Closes #1410

What's New in astroid 2.10.1?
=============================
Expand Down
9 changes: 8 additions & 1 deletion astroid/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class cachedproperty:
After first usage, the <property_name> becomes part of the object's
__dict__. Doing:

del obj.<property_name> empties the cache.
del obj.<property_name> empties the cache.

Idea taken from the pyramid_ framework and the mercurial_ project.

Expand All @@ -70,6 +70,13 @@ class cachedproperty:
__slots__ = ("wrapped",)

def __init__(self, wrapped):
if sys.version_info >= (3, 8):
warnings.warn(
"The cachedproperty class is functionally the same as functools.cached_property "
"and will be removed after support for Python < 3.8 has been dropped. "
"Consider importing the functools variant on >= 3.8.",
Copy link
Member

Choose a reason for hiding this comment

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

I would actually suggest to remove it with astroid 3.0 (at least for Python 3.8+). We can define it conditionally after that point.

if sys.version_info < (3, 8):
    class cachedproperty:
        ...

My suggestion

cachedproperty has been deprecated and will be removed in astroid 3.0 for Python 3.8+.
Use functools.cached_property instead.

--
As a side note, personally I do consider cachedproperty to be internal. Thus the deprecation note is more for us than anyone else. It might even make sense to add a TODO along the lines: Remove completely when support for 3.7 is dropped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't know if 3.0 would be 3.8+ so I didn't mention a specific version. I'll update.

DeprecationWarning,
)
try:
wrapped.__name__
except AttributeError as exc:
Expand Down
10 changes: 8 additions & 2 deletions astroid/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"""This module contains some mixins for the different nodes.
"""
import itertools
import sys
from typing import TYPE_CHECKING, Optional

from astroid import decorators
Expand All @@ -26,11 +27,16 @@
if TYPE_CHECKING:
from astroid import nodes

if sys.version_info >= (3, 8) or TYPE_CHECKING:
from functools import cached_property
else:
from astroid.decorators import cachedproperty as cached_property


class BlockRangeMixIn:
"""override block range"""

@decorators.cachedproperty
@cached_property
def blockstart_tolineno(self):
return self.lineno

Expand Down Expand Up @@ -135,7 +141,7 @@ class MultiLineBlockMixin:
Assign nodes, etc.
"""

@decorators.cachedproperty
@cached_property
def _multi_line_blocks(self):
return tuple(getattr(self, field) for field in self._multi_line_block_fields)

Expand Down
21 changes: 13 additions & 8 deletions astroid/nodes/node_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@
from astroid import nodes
from astroid.nodes import LocalsDictNodeNG

if sys.version_info >= (3, 8) or TYPE_CHECKING:
from functools import cached_property # pylint: disable=ungrouped-imports
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from functools import cached_property # pylint: disable=ungrouped-imports
# pylint: disable-next=ungrouped-imports
from functools import cached_property

This looks like something we should fix in pylint -> ungrouped-imports 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah something weird is going on here..

else:
from astroid.decorators import cachedproperty as cached_property


def _is_const(value):
return isinstance(value, tuple(CONST_CLS))
Expand Down Expand Up @@ -824,7 +829,7 @@ def _infer_name(self, frame, name):
return name
return None

@decorators.cachedproperty
@cached_property
def fromlineno(self):
"""The first line that this node appears on in the source code.

Expand All @@ -833,7 +838,7 @@ def fromlineno(self):
lineno = super().fromlineno
return max(lineno, self.parent.fromlineno or 0)

@decorators.cachedproperty
@cached_property
def arguments(self):
"""Get all the arguments for this node, including positional only and positional and keyword"""
return list(itertools.chain((self.posonlyargs or ()), self.args or ()))
Expand Down Expand Up @@ -2601,7 +2606,7 @@ def postinit(
if body is not None:
self.body = body

@decorators.cachedproperty
@cached_property
def blockstart_tolineno(self):
"""The line on which the beginning of this block ends.

Expand Down Expand Up @@ -2734,7 +2739,7 @@ def postinit(
See astroid/protocols.py for actual implementation.
"""

@decorators.cachedproperty
@cached_property
def blockstart_tolineno(self):
"""The line on which the beginning of this block ends.

Expand Down Expand Up @@ -3093,7 +3098,7 @@ def postinit(
if isinstance(self.parent, If) and self in self.parent.orelse:
self.is_orelse = True

@decorators.cachedproperty
@cached_property
def blockstart_tolineno(self):
"""The line on which the beginning of this block ends.

Expand Down Expand Up @@ -3762,7 +3767,7 @@ def _wrap_attribute(self, attr):
return const
return attr

@decorators.cachedproperty
@cached_property
def _proxied(self):
builtins = AstroidManager().builtins_module
return builtins.getattr("slice")[0]
Expand Down Expand Up @@ -4384,7 +4389,7 @@ def postinit(
if orelse is not None:
self.orelse = orelse

@decorators.cachedproperty
@cached_property
def blockstart_tolineno(self):
"""The line on which the beginning of this block ends.

Expand Down Expand Up @@ -4500,7 +4505,7 @@ def postinit(
See astroid/protocols.py for actual implementation.
"""

@decorators.cachedproperty
@cached_property
def blockstart_tolineno(self):
"""The line on which the beginning of this block ends.

Expand Down
9 changes: 7 additions & 2 deletions astroid/nodes/node_ng.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
else:
from typing_extensions import Literal

if sys.version_info >= (3, 8) or TYPE_CHECKING:
from functools import cached_property # pylint: disable=ungrouped-imports
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from functools import cached_property # pylint: disable=ungrouped-imports
# pylint: disable-next=ungrouped-imports
from functools import cached_property

else:
# pylint: disable-next=ungrouped-imports
from astroid.decorators import cachedproperty as cached_property

# Types for 'NodeNG.nodes_of_class()'
T_Nodes = TypeVar("T_Nodes", bound="NodeNG")
Expand Down Expand Up @@ -435,14 +440,14 @@ def previous_sibling(self):
# these are lazy because they're relatively expensive to compute for every
# single node, and they rarely get looked at

@decorators.cachedproperty
@cached_property
def fromlineno(self) -> Optional[int]:
"""The first line that this node appears on in the source code."""
if self.lineno is None:
return self._fixed_source_line()
return self.lineno

@decorators.cachedproperty
@cached_property
def tolineno(self) -> Optional[int]:
"""The last line that this node appears on in the source code."""
if self.end_lineno is not None:
Expand Down
20 changes: 13 additions & 7 deletions astroid/nodes/scoped_nodes/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
import sys
import typing
import warnings
from typing import Dict, List, Optional, Set, TypeVar, Union, overload
from typing import TYPE_CHECKING, Dict, List, Optional, Set, TypeVar, Union, overload

from astroid import bases
from astroid import decorators as decorators_mod
Expand Down Expand Up @@ -93,6 +93,12 @@
else:
from typing_extensions import Literal

if sys.version_info >= (3, 8) or TYPE_CHECKING:
from functools import cached_property
else:
# pylint: disable-next=ungrouped-imports
from astroid.decorators import cachedproperty as cached_property


ITER_METHODS = ("__iter__", "__getitem__")
EXCEPTION_BASE_CLASSES = frozenset({"Exception", "BaseException"})
Expand Down Expand Up @@ -1611,7 +1617,7 @@ def postinit(
self.position = position
self.doc_node = doc_node

@decorators_mod.cachedproperty
@cached_property
def extra_decorators(self) -> List[node_classes.Call]:
"""The extra decorators that this function can have.

Expand Down Expand Up @@ -1652,7 +1658,7 @@ def extra_decorators(self) -> List[node_classes.Call]:
decorators.append(assign.value)
return decorators

@decorators_mod.cachedproperty
@cached_property
def type(
self,
): # pylint: disable=invalid-overridden-method,too-many-return-statements
Expand Down Expand Up @@ -1726,7 +1732,7 @@ def type(
pass
return type_name

@decorators_mod.cachedproperty
@cached_property
def fromlineno(self) -> Optional[int]:
"""The first line that this node appears on in the source code."""
# lineno is the line number of the first decorator, we want the def
Expand All @@ -1739,7 +1745,7 @@ def fromlineno(self) -> Optional[int]:

return lineno

@decorators_mod.cachedproperty
@cached_property
def blockstart_tolineno(self):
"""The line on which the beginning of this block ends.

Expand Down Expand Up @@ -2337,7 +2343,7 @@ def _newstyle_impl(self, context=None):
doc=("Whether this is a new style class or not\n\n" ":type: bool or None"),
)

@decorators_mod.cachedproperty
@cached_property
def fromlineno(self) -> Optional[int]:
"""The first line that this node appears on in the source code."""
if not PY38_PLUS:
Expand All @@ -2352,7 +2358,7 @@ def fromlineno(self) -> Optional[int]:
return lineno
return super().fromlineno

@decorators_mod.cachedproperty
@cached_property
def blockstart_tolineno(self):
"""The line on which the beginning of this block ends.

Expand Down
15 changes: 11 additions & 4 deletions astroid/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
Call(func=Name('frozenset'), args=Tuple(...))
"""

import sys
from typing import TYPE_CHECKING

from astroid import bases, decorators, util
from astroid import bases, util
from astroid.exceptions import (
AttributeInferenceError,
InferenceError,
Expand All @@ -35,6 +37,11 @@

objectmodel = util.lazy_import("interpreter.objectmodel")

if sys.version_info >= (3, 8) or TYPE_CHECKING:
from functools import cached_property
else:
from astroid.decorators import cachedproperty as cached_property
Comment on lines +40 to +43
Copy link
Member

Choose a reason for hiding this comment

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

Strange that pylint: disable-next=ungrouped-imports is needed in some cases but not all 🤔



class FrozenSet(node_classes.BaseContainer):
"""class representing a FrozenSet composite node"""
Expand All @@ -45,7 +52,7 @@ def pytype(self):
def _infer(self, context=None):
yield self

@decorators.cachedproperty
@cached_property
def _proxied(self): # pylint: disable=method-hidden
ast_builtins = AstroidManager().builtins_module
return ast_builtins.getattr("frozenset")[0]
Expand Down Expand Up @@ -114,7 +121,7 @@ def super_mro(self):
index = mro.index(self.mro_pointer)
return mro[index + 1 :]

@decorators.cachedproperty
@cached_property
def _proxied(self):
ast_builtins = AstroidManager().builtins_module
return ast_builtins.getattr("super")[0]
Expand Down Expand Up @@ -218,7 +225,7 @@ class ExceptionInstance(bases.Instance):
the case of .args.
"""

@decorators.cachedproperty
@cached_property
def special_attributes(self):
qname = self.qname()
instance = objectmodel.BUILTIN_EXCEPTIONS.get(
Expand Down
19 changes: 18 additions & 1 deletion tests/unittest_decorators.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import sys

import pytest
from _pytest.recwarn import WarningsRecorder

from astroid.decorators import deprecate_default_argument_values
from astroid.decorators import cachedproperty, deprecate_default_argument_values


class SomeClass:
Expand Down Expand Up @@ -97,3 +99,18 @@ def test_deprecated_default_argument_values_ok(recwarn: WarningsRecorder) -> Non
instance = SomeClass(name="some_name")
instance.func(name="", var=42)
assert len(recwarn) == 0


@pytest.mark.skipif(sys.version_info < (3, 8), reason="Requires Python 3.8 or higher")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@pytest.mark.skipif(sys.version_info < (3, 8), reason="Requires Python 3.8 or higher")
@pytest.mark.skipif(not PY38_PLUS, reason="Requires Python 3.8 or higher")

For tests it isn't necessary to use sys.version_info (for typing).

def test_deprecation_warning_on_cachedproperty() -> None:
"""Check the DeprecationWarning on cachedproperty."""

with pytest.warns(DeprecationWarning) as records:

class MyClass: # pylint: disable=unused-variable
@cachedproperty
def my_property(self):
return 1

assert len(records) == 1
assert "functools.cached_property" in records[0].message.args[0]