Skip to content

CLN: Rolling helper attributes #38664

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 23, 2020
Merged

CLN: Rolling helper attributes #38664

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 22 additions & 36 deletions pandas/core/window/rolling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from pandas._typing import ArrayLike, Axis, FrameOrSeries, FrameOrSeriesUnion
from pandas.compat._optional import import_optional_dependency
from pandas.compat.numpy import function as nv
from pandas.util._decorators import Appender, Substitution, cache_readonly, doc
from pandas.util._decorators import Appender, Substitution, doc

from pandas.core.dtypes.common import (
ensure_float64,
Expand Down Expand Up @@ -110,16 +110,23 @@ def __init__(
self.win_type = win_type
self.axis = obj._get_axis_number(axis) if axis is not None else None
self._win_freq_i8 = None
if self.on is None:
if self.axis == 0:
self._on = self.obj.index
else:
# i.e. self.axis == 1
self._on = self.obj.columns
elif isinstance(self.on, Index):
self._on = self.on
elif isinstance(self.obj, ABCDataFrame) and self.on in self.obj.columns:
self._on = Index(self.obj[self.on])
else:
raise ValueError(
f"invalid on specified as {self.on}, "
"must be a column (of DataFrame), an Index or None"
)
self.validate()

@property
def is_datetimelike(self) -> Optional[bool]:
return None

@property
def _on(self):
return None

def validate(self) -> None:
if self.center is not None and not is_bool(self.center):
raise ValueError("center must be a boolean")
Expand Down Expand Up @@ -1822,37 +1829,16 @@ def _get_corr(a, b):


class Rolling(RollingAndExpandingMixin):
@cache_readonly
def is_datetimelike(self) -> bool:
Copy link
Member

@jorisvandenbossche jorisvandenbossche Jan 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Note that those attributes are public (whether we like that or not ..), and so should ideally be deprecated first if we remove them.

Copy link
Member

Choose a reason for hiding this comment

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

(note I was looking at some rolling-related PRs because of a dask failure, see #38641 (comment), but this change about is_datetimelike didn't actually cause an issue in dask, to be clear)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am -1 on deprecating anything here. these are internal and never documented as public.

return isinstance(
self._on, (ABCDatetimeIndex, ABCTimedeltaIndex, ABCPeriodIndex)
)

@cache_readonly
def _on(self) -> Index:
if self.on is None:
if self.axis == 0:
return self.obj.index
else:
# i.e. self.axis == 1
return self.obj.columns
elif isinstance(self.on, Index):
return self.on
elif isinstance(self.obj, ABCDataFrame) and self.on in self.obj.columns:
return Index(self.obj[self.on])
else:
raise ValueError(
f"invalid on specified as {self.on}, "
"must be a column (of DataFrame), an Index or None"
)

def validate(self):
super().validate()

# we allow rolling on a datetimelike index
if (self.obj.empty or self.is_datetimelike) and isinstance(
self.window, (str, BaseOffset, timedelta)
):
if (
self.obj.empty
or isinstance(
self._on, (ABCDatetimeIndex, ABCTimedeltaIndex, ABCPeriodIndex)
)
) and isinstance(self.window, (str, BaseOffset, timedelta)):

self._validate_monotonic()

Expand Down