-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
DEPR: min_periods=None behavior for Rolling.count #36649
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
Changes from 14 commits
424ad42
f7c82f5
4f2c0fc
a210514
f0b34e4
bdd0ef7
42d6cfb
109eb61
c691985
584d0bb
26bc6c0
bcb3be8
8d047d8
b16317d
93601c2
98a8964
94abc99
e020f0a
ff751c1
03ce055
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
Type, | ||
Union, | ||
) | ||
import warnings | ||
|
||
import numpy as np | ||
|
||
|
@@ -469,31 +470,39 @@ def _get_window_indexer(self, window: int) -> BaseIndexer: | |
return VariableWindowIndexer(index_array=self._on.asi8, window_size=window) | ||
return FixedWindowIndexer(window_size=window) | ||
|
||
def _apply_series(self, homogeneous_func: Callable[..., ArrayLike]) -> "Series": | ||
def _apply_series( | ||
self, homogeneous_func: Callable[..., ArrayLike], name: Optional[str] = None | ||
) -> "Series": | ||
""" | ||
Series version of _apply_blockwise | ||
""" | ||
obj = self._create_data(self._selected_obj) | ||
|
||
try: | ||
values = self._prep_values(obj.values) | ||
# GH 12541: Special case for count where we support date-like types | ||
input = obj.values if name != "count" else notna(obj.values).astype(int) | ||
values = self._prep_values(input) | ||
except (TypeError, NotImplementedError) as err: | ||
raise DataError("No numeric types to aggregate") from err | ||
|
||
result = homogeneous_func(values) | ||
return obj._constructor(result, index=obj.index, name=obj.name) | ||
|
||
def _apply_blockwise( | ||
self, homogeneous_func: Callable[..., ArrayLike] | ||
self, homogeneous_func: Callable[..., ArrayLike], name: Optional[str] = None | ||
) -> FrameOrSeriesUnion: | ||
""" | ||
Apply the given function to the DataFrame broken down into homogeneous | ||
sub-frames. | ||
""" | ||
if self._selected_obj.ndim == 1: | ||
return self._apply_series(homogeneous_func) | ||
return self._apply_series(homogeneous_func, name) | ||
|
||
obj = self._create_data(self._selected_obj) | ||
if name == "count": | ||
# GH 12541: Special case for count where we support date-like types | ||
obj = notna(obj).astype(int) | ||
obj._mgr = obj._mgr.consolidate() | ||
mgr = obj._mgr | ||
|
||
def hfunc(bvalues: ArrayLike) -> ArrayLike: | ||
|
@@ -606,7 +615,7 @@ def calc(x): | |
|
||
return result | ||
|
||
return self._apply_blockwise(homogeneous_func) | ||
return self._apply_blockwise(homogeneous_func, name) | ||
|
||
def aggregate(self, func, *args, **kwargs): | ||
result, how = self._aggregate(func, *args, **kwargs) | ||
|
@@ -1265,33 +1274,8 @@ class RollingAndExpandingMixin(BaseWindow): | |
) | ||
|
||
def count(self): | ||
# GH 32865. Using count with custom BaseIndexer subclass | ||
# implementations shouldn't end up here | ||
assert not isinstance(self.window, BaseIndexer) | ||
|
||
obj = self._create_data(self._selected_obj) | ||
|
||
def hfunc(values: np.ndarray) -> np.ndarray: | ||
result = notna(values) | ||
result = result.astype(int) | ||
frame = type(obj)(result.T) | ||
result = self._constructor( | ||
frame, | ||
window=self._get_window(), | ||
min_periods=self.min_periods or 0, | ||
center=self.center, | ||
axis=self.axis, | ||
closed=self.closed, | ||
).sum() | ||
return result.values.T | ||
|
||
new_mgr = obj._mgr.apply(hfunc) | ||
out = obj._constructor(new_mgr) | ||
if obj.ndim == 1: | ||
out.name = obj.name | ||
else: | ||
self._insert_on_column(out, obj) | ||
return out | ||
window_func = self._get_cython_func_type("roll_sum") | ||
return self._apply(window_func, center=self.center, name="count") | ||
|
||
_shared_docs["apply"] = dedent( | ||
r""" | ||
|
@@ -2050,14 +2034,16 @@ def aggregate(self, func, *args, **kwargs): | |
@Substitution(name="rolling") | ||
@Appender(_shared_docs["count"]) | ||
def count(self): | ||
|
||
# different impl for freq counting | ||
# GH 32865. Use a custom count function implementation | ||
# when using a BaseIndexer subclass as a window | ||
if self.is_freq_type or isinstance(self.window, BaseIndexer): | ||
window_func = self._get_roll_func("roll_count") | ||
return self._apply(window_func, center=self.center, name="count") | ||
|
||
if self.min_periods is None: | ||
warnings.warn( | ||
( | ||
"min_periods=None will default to the size of window " | ||
"consistent with other methods in a future version. " | ||
"Specify min_periods=0 instead." | ||
), | ||
DeprecationWarning, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this a FutureWarning. I know its loud but i think we need to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alt we could do this for a version and then change it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to FutureWarning |
||
) | ||
self.min_periods = 0 | ||
return super().count() | ||
|
||
@Substitution(name="rolling") | ||
|
Uh oh!
There was an error while loading. Please reload this page.