-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
[ArrayManager] Remaining GroupBy tests (fix count, pass on libreduction for now) #40050
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 1 commit
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 |
---|---|---|
|
@@ -270,15 +270,30 @@ def grouped_reduce(self: T, func: Callable, ignore_failures: bool = False) -> T: | |
------- | ||
ArrayManager | ||
""" | ||
# TODO ignore_failures | ||
result_arrays = [func(arr) for arr in self.arrays] | ||
result_arrays: List[np.ndarray] = [] | ||
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. this looks a whole lot like reduce right? 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. Yes, it's quite similar in logic (it are also both reduce operations, so not that unsurprising), but IMO they are different enough that trying to share anything will only make it more complex (return value is different, they need to process the result inside the loop differently, etc) It might be possible to change the return value of |
||
result_indices: List[int] = [] | ||
|
||
for i, arr in enumerate(self.arrays): | ||
try: | ||
res = func(arr) | ||
except (TypeError, NotImplementedError): | ||
if not ignore_failures: | ||
raise | ||
continue | ||
result_arrays.append(res) | ||
result_indices.append(i) | ||
|
||
if len(result_arrays) == 0: | ||
index = Index([None]) # placeholder | ||
else: | ||
index = Index(range(result_arrays[0].shape[0])) | ||
|
||
return type(self)(result_arrays, [index, self.items]) | ||
if ignore_failures: | ||
columns = self.items[np.array(result_indices, dtype="int64")] | ||
else: | ||
columns = self.items | ||
|
||
return type(self)(result_arrays, [index, columns]) | ||
|
||
def operate_blockwise(self, other: ArrayManager, array_op) -> ArrayManager: | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
import numpy as np | ||
import pytest | ||
|
||
import pandas.util._test_decorators as td | ||
|
||
from pandas.core.dtypes.common import ( | ||
ensure_platform_int, | ||
is_timedelta64_dtype, | ||
|
@@ -161,8 +163,11 @@ def test_transform_broadcast(tsframe, ts): | |
assert_fp_equal(res.xs(idx), agged[idx]) | ||
|
||
|
||
def test_transform_axis_1(request, transformation_func): | ||
def test_transform_axis_1(request, transformation_func, using_array_manager): | ||
# GH 36308 | ||
if using_array_manager and transformation_func == "pct_change": | ||
# TODO(ArrayManager) column-wise shift | ||
pytest.skip("ArrayManager: column-wise not yet implemented") | ||
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. xfail? 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. Yes, changed |
||
warn = None | ||
if transformation_func == "tshift": | ||
warn = FutureWarning | ||
|
@@ -183,6 +188,8 @@ def test_transform_axis_1(request, transformation_func): | |
tm.assert_equal(result, expected) | ||
|
||
|
||
# TODO(ArrayManager) groupby().transform returns DataFrame backed by BlockManager | ||
@td.skip_array_manager_not_yet_implemented | ||
def test_transform_axis_ts(tsframe): | ||
|
||
# make sure that we are setting the axes | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im totally fine with skipping for the time being. medium-term, i think it could use .arrays instead of .blocks, might be easy-ish compat