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

Replace custom cached_property implementation with functools #10272

Merged
merged 4 commits into from
Feb 14, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion python/cudf/cudf/core/_base_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import pickle
from functools import cached_property
from typing import Any, Set

import pandas as pd
Expand Down Expand Up @@ -31,7 +32,6 @@
is_mixed_with_object_dtype,
numeric_normalize_types,
)
from cudf.utils.utils import cached_property


class BaseIndex(Serializable):
Expand Down
7 changes: 3 additions & 4 deletions python/cudf/cudf/core/column_accessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import itertools
from collections.abc import MutableMapping
from functools import reduce
from functools import cached_property, reduce
from typing import (
TYPE_CHECKING,
Any,
Expand All @@ -20,7 +20,6 @@

import cudf
from cudf.core import column
from cudf.utils.utils import cached_property

if TYPE_CHECKING:
from cudf.core.column import ColumnBase
Expand Down Expand Up @@ -360,9 +359,9 @@ def select_by_index(self, index: Any) -> ColumnAccessor:
start, stop, step = index.indices(len(self._data))
keys = self.names[start:stop:step]
elif pd.api.types.is_integer(index):
keys = [self.names[index]]
keys = (self.names[index],)
else:
keys = (self.names[i] for i in index)
keys = tuple(self.names[i] for i in index)
data = {k: self._data[k] for k in keys}
return self.__class__(
data, multiindex=self.multiindex, level_names=self.level_names,
Expand Down
4 changes: 2 additions & 2 deletions python/cudf/cudf/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def _num_rows(self) -> int:

@property
def _column_names(self) -> List[Any]: # TODO: List[str]?
return self._data.names
return list(self._data.names)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We previously returned a tuple while hinting that we were returning a list. Which one should we return?

Copy link
Contributor

Choose a reason for hiding this comment

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

From the perspective of immutability, a tuple. However, I have come across a few developer mistakes where the result of this property was used as if it were a list, perhaps because of the incorrect typing. cc: @isVoid who ran into this with me in this commit: a4d88c3


@property
def _index_names(self) -> List[Any]: # TODO: List[str]?
Expand All @@ -105,7 +105,7 @@ def _index_names(self) -> List[Any]: # TODO: List[str]?

@property
def _columns(self) -> List[Any]: # TODO: List[Column]?
return self._data.columns
return list(self._data.columns)

def serialize(self):
header = {
Expand Down
3 changes: 2 additions & 1 deletion python/cudf/cudf/core/groupby/groupby.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import itertools
import pickle
import warnings
from functools import cached_property

import numpy as np
import pandas as pd
Expand All @@ -16,7 +17,7 @@
from cudf.core.abc import Serializable
from cudf.core.column.column import arange, as_column
from cudf.core.multiindex import MultiIndex
from cudf.utils.utils import GetAttrGetItemMixin, cached_property
from cudf.utils.utils import GetAttrGetItemMixin


# The three functions below return the quantiles [25%, 50%, 75%]
Expand Down
3 changes: 2 additions & 1 deletion python/cudf/cudf/core/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import math
import pickle
import warnings
from functools import cached_property
from numbers import Number
from typing import (
Any,
Expand Down Expand Up @@ -54,7 +55,7 @@
from cudf.core.single_column_frame import SingleColumnFrame
from cudf.utils.docutils import copy_docstring
from cudf.utils.dtypes import find_common_type
from cudf.utils.utils import cached_property, search_range
from cudf.utils.utils import search_range

T = TypeVar("T", bound="Frame")

Expand Down
2 changes: 1 addition & 1 deletion python/cudf/cudf/core/indexed_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import operator
import warnings
from collections import Counter, abc
from functools import cached_property
from typing import Callable, Type, TypeVar
from uuid import uuid4

Expand All @@ -30,7 +31,6 @@
from cudf.core.index import Index, RangeIndex, _index_from_columns
from cudf.core.multiindex import MultiIndex
from cudf.core.udf.utils import _compile_or_get, _supported_cols_from_frame
from cudf.utils.utils import cached_property

doc_reset_index_template = """
Reset the index of the {klass}, or a level of it.
Expand Down
4 changes: 2 additions & 2 deletions python/cudf/cudf/core/join/join.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright (c) 2020-2021, NVIDIA CORPORATION.
from __future__ import annotations

from typing import TYPE_CHECKING, Callable, cast
from typing import TYPE_CHECKING, Any, Callable, List, cast

import cudf
from cudf import _lib as libcudf
Expand Down Expand Up @@ -320,7 +320,7 @@ def _sort_result(self, result: Frame) -> Frame:
# same order as given in 'on'. If the indices are used as
# keys, the index will be sorted. If one index is specified,
# the key columns on the other side will be used to sort.
by = []
by: List[Any] = []
if self._using_left_index and self._using_right_index:
if result._index is not None:
by.extend(result._index._data.columns)
Expand Down
7 changes: 2 additions & 5 deletions python/cudf/cudf/core/multiindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import numbers
import pickle
from collections.abc import Sequence
from functools import cached_property
from numbers import Integral
from typing import Any, List, MutableMapping, Optional, Tuple, Union

Expand All @@ -22,11 +23,7 @@
from cudf.core._compat import PANDAS_GE_120
from cudf.core.frame import Frame
from cudf.core.index import BaseIndex, _lexsorted_equal_range, as_index
from cudf.utils.utils import (
NotIterable,
_maybe_indices_to_slice,
cached_property,
)
from cudf.utils.utils import NotIterable, _maybe_indices_to_slice


class MultiIndex(Frame, BaseIndex, NotIterable):
Expand Down
22 changes: 0 additions & 22 deletions python/cudf/cudf/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,28 +144,6 @@ def set_allocator(
IS_NEP18_ACTIVE = _is_nep18_active()


class cached_property:
"""
Like @property, but only evaluated upon first invocation.
To force re-evaluation of a cached_property, simply delete
it with `del`.
"""

# TODO: Can be replaced with functools.cached_property when we drop support
# for Python 3.7.

def __init__(self, func):
self.func = func

def __get__(self, instance, cls):
if instance is None:
return self
else:
value = self.func(instance)
object.__setattr__(instance, self.func.__name__, value)
return value


class GetAttrGetItemMixin:
"""This mixin changes `__getattr__` to attempt a `__getitem__` call.

Expand Down