BUG/TST: non-numeric EA reductions by lukemanley · Pull Request #59234 · pandas-dev/pandas (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits4 Checks37 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

lukemanley

Fixes a few bugs related to non-numeric EA reductions and expands testing by un-skipping a number of existing tests.

> pytest -k test_reduce pandas/tests/extension/

# main branch
2160 passed, 1222 skipped, 154 xfailed

# PR
2507 passed, 886 skipped, 143 xfailed

BUG fix example:

In [1]: import pandas as pd

In [2]: arr = pd.array([None], dtype="duration[ms][pyarrow]")

In [3]: pd.DataFrame({"a": arr}).min().dtype

# main branch
Out[3]: int64[pyarrow]

# PR
Out[3]: duration[ms][pyarrow]

@lukemanley

@lukemanley

mroeschke

@@ -525,7 +525,10 @@ def _reduce(
self, name: str, *, skipna: bool = True, axis: AxisInt | None = 0, **kwargs
):
if name in ["min", "max"]:
return getattr(self, name)(skipna=skipna, axis=axis)
result = getattr(self, name)(skipna=skipna, axis=axis)
if kwargs.get("keepdims"):

Choose a reason for hiding this comment

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

Could you include keepdims in the _reduce signature instead?

Choose a reason for hiding this comment

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

updated

mroeschke

if name == "std":
from pandas.core.arrays import TimedeltaArray
return TimedeltaArray._from_sequence(result)

Choose a reason for hiding this comment

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

Does this possibly preserve the resolution of the input? e.g std of a ms resolution datetime array should return ms resolution

Choose a reason for hiding this comment

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

yes, the resolution is preserved in nanops.nanstd.

@lukemanley

@lukemanley

mroeschke

@mroeschke

lithomas1 added a commit to lithomas1/pandas that referenced this pull request

Sep 9, 2024

@lithomas1

2 participants

@lukemanley @mroeschke