Skip to content

Implement some reductions for string Series #31757

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

Closed
dsaxton wants to merge 30 commits into from
Closed

Implement some reductions for string Series #31757

Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c87ce47
Add reduction tests
Feb 6, 2020
5365fa1
Implement a few reductions
Feb 6, 2020
460030f
Fix test
Feb 6, 2020
6497e16
Add whatsnew
Feb 6, 2020
c052feb
skipna
Feb 6, 2020
4312b4f
Hardcode expected output
Feb 11, 2020
b67cc56
Merge branch 'master' into reduce-string
Feb 11, 2020
214995f
Skip tests
Feb 12, 2020
95c83fb
Merge branch 'master' into reduce-string
Feb 13, 2020
4082ae3
No else
Feb 13, 2020
12a2323
Modify nanminmax to better handle object dtype
Feb 14, 2020
4a61b81
Delegate to min and max methods
Feb 14, 2020
c6465e5
Fixup nanops
Feb 14, 2020
5c1a5fb
Import and clean up
Feb 14, 2020
ca99650
Update tests
Feb 14, 2020
18800e1
Update whatsnew
Feb 14, 2020
3eddb73
Limit to values.ndim == 1
Feb 14, 2020
0c1a2a3
Sort index in test
Feb 14, 2020
5714c06
Box in Series
Feb 14, 2020
5e01c3f
Add Series min / max test
Feb 14, 2020
0e4a4e2
Merge branch 'master' into reduce-string
Feb 14, 2020
ed01138
Move tests
Feb 14, 2020
1128950
Pass in mask and patch _maybe_get_mask
Feb 15, 2020
7c0f05d
Update whatsnew
Feb 15, 2020
d84f5bd
Merge branch 'master' into reduce-string
Feb 15, 2020
7db2052
Try to make mypy happy
Feb 15, 2020
e19bbe1
Update doc/source/whatsnew/v1.0.2.rst
dsaxton Feb 15, 2020
df99b4f
Add comment
Feb 15, 2020
ba20705
Merge branch 'master' into reduce-string
Feb 15, 2020
d69b587
Merge remote-tracking branch 'upstream/master' into reduce-string
dsaxton Feb 25, 2020
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
Prev Previous commit
Next Next commit
Modify nanminmax to better handle object dtype
  • Loading branch information
Daniel Saxton authored and Daniel Saxton committed Feb 14, 2020
commit 12a232344a70239e16e4bc3a92f24dcfd06c9df4
8 changes: 8 additions & 0 deletions pandas/core/nanops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,8 @@ def reduction(
mask: Optional[np.ndarray] = None,
) -> Dtype:

na_mask = isna(values)
Copy link
Contributor

@jreback jreback Feb 15, 2020

Choose a reason for hiding this comment

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

you should already have the mask (pass it in when you call this).


values, mask, dtype, dtype_max, fill_value = _get_values(
values, skipna, fill_value_typ=fill_value_typ, mask=mask
)
Expand All @@ -864,6 +866,12 @@ def reduction(
result.fill(np.nan)
except (AttributeError, TypeError, ValueError):
result = np.nan
elif is_object_dtype(dtype) and na_mask.any():
# Need to explicitly mask NA values for object dtypes
Copy link
Contributor

@jreback jreback Feb 16, 2020

Choose a reason for hiding this comment

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

why?

if skipna:
result = getattr(values[~na_mask], meth)(axis)
Copy link
Member

@jorisvandenbossche jorisvandenbossche Feb 14, 2020

Choose a reason for hiding this comment

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

This masking could also be done in the min/max functions? (as you had before?)

Or, another option might be to add a min/max function to mask_ops.py, similarly as I am doing for sum in #30982 (but it should be simpler for min/max, as those don't need to handle the min_count)

Copy link
Contributor Author

@dsaxton dsaxton Feb 14, 2020

Choose a reason for hiding this comment

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

I think a benefit of having it here is that this also fixes a bug for Series: pd.Series(["a", np.nan]).min() currently raises even though it shouldn't

Copy link
Member

@jorisvandenbossche jorisvandenbossche Feb 14, 2020

Choose a reason for hiding this comment

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

Ah, that's a good point. Can you add a test for that, then?

Now, that aside, I think longer term we still want the separate min/max in mask_ops.py, so it can also be used for the int dtypes. But that can then certainly be done for a separate PR.

else:
return np.nan
else:
result = getattr(values, meth)(axis)

Expand Down