ENH: ArrayManager.quantile #40189
ENH: ArrayManager.quantile #40189
Conversation
| ) -> ArrayManager: | ||
|
|
||
| arrs = [ | ||
| x if not isinstance(x, np.ndarray) else np.atleast_2d(x) |
There was a problem hiding this comment.
Can the reshaping be moved to quantile_compat? (as it also already does this for EAs)
There was a problem hiding this comment.
this one (potentially)
There was a problem hiding this comment.
i think ArrayManager-specific logic should stay in ArrayManager wherever possible
| @@ -526,6 +524,7 @@ def test_quantile_empty_no_columns(self): | |||
| expected.columns.name = "captain tightpants" | |||
| tm.assert_frame_equal(result, expected) | |||
|
|
|||
| @td.skip_array_manager_invalid_test | |||
| def test_quantile_item_cache(self): | |||
| # previous behavior incorrect retained an invalid _item_cache entry | |||
| df = DataFrame(np.random.randn(4, 3), columns=["A", "B", "C"]) | |||
There was a problem hiding this comment.
I can't comment on the exact line (3 lines below), but could also do
if not using_array_manager:
assert len(df._mgr.blocks) == 2
because I think the rest of the test is still valid
| @@ -554,13 +554,6 @@ def quantile( | |||
| for blk in self.blocks | |||
| ] | |||
|
|
|||
| if transposed: | |||
There was a problem hiding this comment.
The transposed keyword is no longer used after removing this? (so the keyword can be removed as well)
| def test_quantile_ea_all_na(self, index, frame_or_series): | ||
|
|
||
| obj = frame_or_series(index).copy() | ||
|
|
||
| obj.iloc[:] = index._na_value | ||
|
|
||
| # TODO: this casting should be unnecessary after GH#39763 is fixed |
There was a problem hiding this comment.
| # TODO: this casting should be unnecessary after GH#39763 is fixed | |
| # TODO(ArrayManager) this casting should be unnecessary after GH#39763 is fixed |
|
comments addressed i think |
|
|
||
|
|
||
| def quantile_ea_compat( | ||
| values: ExtensionArray, qs, interpolation: str, axis: int |
There was a problem hiding this comment.
this is all super gross. i guess moving it here and then going to clean in future?
There was a problem hiding this comment.
depends on a few things: this can become somewhat less gross with 2D EAs (or 1D ndarrays in an ArrayManager world). There's also the fact that values_for_factorize used here doesnt work for IntegerArray/FloatingArray.
There was a problem hiding this comment.
can this be better if we expose a .quantile() on EAs?
There was a problem hiding this comment.
that may eventually be the way to go. will see if thats what it takes to get the IntegerArray/FloatingArray working again
|
can you merge master |
rebased+greenish |
xref #39146