Support `numeric_only` for simple groupby aggregations for `pandas` 2.0 compatibility #9889

j-bennet · 2023-01-27T17:04:55Z

Partially implement numeric_only on GroupBy operations, to align the behavior with Pandas.

This PR only includes changes for aggs that are using _single_agg internally. More complicated aggs will have to be handled separately.

Xref #9736.
Xref #9471.

Tests added / passed
Passes pre-commit run --all-files

…by-numeric-only

j-bennet · 2023-01-27T19:50:11Z

There's one failure left to resolve, dask/dataframe/tests/test_groupby.py::test_groupby_numeric_only_supported fails with sum agg and numeric_only=False with pandas 1.5:

            elif isinstance(dsk, dd.DataFrame):
                assert "DataFrame" in type(result).__name__, type(result)
                assert isinstance(dsk.columns, pd.Index), type(dsk.columns)
                assert type(dsk._meta) == type(result), type(dsk._meta)
                if check_names:
>                   tm.assert_index_equal(dsk.columns, result.columns)
E                   AssertionError: Index are different
E                   
E                   Index length are different
E                   [left]:  2, Index(['B', 'C'], dtype='object')
E                   [right]: 1, Index(['B'], dtype='object')

../utils.py:494: AssertionError

j-bennet · 2023-01-27T21:09:39Z

The test failure is resolved by 8b877b5.

jrbourbeau

Thanks @j-bennet! This will be nice to have -- looking forward to seeing it merged

dask/dataframe/groupby.py

dask/dataframe/tests/test_groupby.py

…by-numeric-only

…ception was raised.

jrbourbeau · 2023-02-02T18:23:29Z

dask/dataframe/_compat.py

        with warnings.catch_warnings():
            warnings.filterwarnings(
                "ignore",
-                message="The default value of numeric_only in",
+                message="Dropping of nuisance columns",


Where's this coming from?

In pandas 1.3, there's this warning on non-numeric data with some of the aggs, and it's different from what it does in 1.5. We caught the 1.5 warning before, but not this one.

@jrbourbeau So we still need this one?

There was one test where this is still needed. I'm inclined to just handle it as follow-up work

jrbourbeau · 2023-02-02T18:30:18Z

dask/dataframe/groupby.py

+            maybe_raise = not (
+                func.__name__ == "agg"
+                and len(args) > 0
+                and args[0] not in NUMERIC_ONLY_NOT_IMPLEMENTED
+            )


Noting that this is to catch when operations in NUMERIC_ONLY_NOT_IMPLEMENTED are being used inside an agg(...) call

jrbourbeau · 2023-02-02T18:34:10Z

dask/dataframe/groupby.py

+    with warnings.catch_warnings():
+        warnings.filterwarnings(
+            "ignore",
+            message="In a future version, the Index constructor will not infer numeric dtypes",
+            category=FutureWarning,
+        )


Is this related to other changes in this PR or known flaky tests?

This makes a flaky test happy.

dask/dataframe/tests/test_arithmetics_reduction.py

dask/dataframe/tests/test_categorical.py

jrbourbeau · 2023-02-02T20:02:46Z

dask/dataframe/tests/test_categorical.py

+        assert_eq(ddf.groupby(ddf.w).y.nunique(), df.groupby(df.w).y.nunique())
+        assert_eq(ddf.y.groupby(ddf.w).count(), df.y.groupby(df.w).count())


I'm curious why these are indented. Are we emitting warnings now? I would have expected us to match warnings from pandas and, since pandas didn't appear to be warning before, I'm confused why we might be

Pandas was warning before, we didn't. Now we do, have to catch it.

dask/dataframe/tests/test_dataframe.py

dask/dataframe/tests/test_groupby.py

…heck warning behavior.

Co-authored-by: James Bourbeau <[email protected]>

j-bennet · 2023-02-03T00:35:01Z

The last failure with minimal dependencies does not seem to be related... possibly flaky?

https://github.com/dask/dask/actions/runs/4079092927/jobs/7030092277

FAILED dask/dataframe/tests/test_arithmetics_reduction.py::test_reductions_frame_dtypes[True-min-None] - AssertionError: Attributes of Series are different

Attribute "dtype" are different
[left]:  object
[right]: float64
FAILED dask/dataframe/tests/test_arithmetics_reduction.py::test_reductions_frame_dtypes[True-max-None] - AssertionError: Attributes of Series are different

Attribute "dtype" are different
[left]:  object
[right]: float64

jrbourbeau · 2023-02-03T18:42:48Z

The last failure with minimal dependencies does not seem to be related... possibly flaky?

Hmm I've not seen those before. My guess is they're somehow related to the changes in this PR

jrbourbeau

Turns out there were a couple of bugs in older versions of pandas that weren't straightforward to workaround. I pushed a9e27d9 which just skips those specific configurations for now. I'll also push up a PR that bumps our minimum pandas version (it's been a while since we've done that).

…6-groupby-numeric-only

j-bennet · 2023-02-09T17:36:24Z

dask/dataframe/tests/test_categorical.py

@@ -117,7 +117,38 @@ def test_concat_unions_categoricals():
    tm.assert_frame_equal(_concat(frames5), pd.concat(frames6))


-def test_unknown_categoricals(shuffle_method):
+# TODO: Remove the filterwarnings below


When can we do this TODO?

jrbourbeau

Thanks @j-bennet!

This PR adds `dtypes` property to `GroupBy`, this will also fix some upstream dask breaking changes introduced in: dask/dask#9889 Issue was discovered in: #12768 (comment) Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Ashwin Srinath (https://github.com/shwina) URL: #12783

numeric_only support for simple aggs, WIP.

6c33a0a

github-actions bot added the dataframe label Jan 27, 2023

j-bennet added 6 commits January 27, 2023 09:18

Merge remote-tracking branch 'upstream/main' into j-bennet/9736-group…

48d2203

…by-numeric-only

Use the same warning behavior as pandas with default numeric_only.

9a17e68

test-upstream

56de96f

Check names in test too.

0acb5af

Make sure we raise when numeric_only=False is not supported.

01f3c65

test-upstream

b5558eb

We need to pass numeric_only to agg function as well.

8b877b5

j-bennet changed the title ~~[WIP] numeric_only support for simple aggs, pandas 2.0 compatibility~~ numeric_only support for simple aggs, pandas 2.0 compatibility Jan 27, 2023

j-bennet requested a review from jrbourbeau January 27, 2023 21:10

test-upstream

7c87fe6

jrbourbeau reviewed Jan 30, 2023

View reviewed changes

j-bennet mentioned this pull request Jan 30, 2023

Add numeric_only to other groupby aggs, pandas 2.0 compatibility #9898

Closed

10 tasks

Irina Truong and others added 13 commits January 31, 2023 10:00

Merge remote-tracking branch 'upstream/main' into j-bennet/9736-group…

489b8f4

…by-numeric-only

Review feedback.

4ef474a

Minor updates

9d314d2

Fix failing tests.

46cd7b4

Small change in test to make sure we test warning context, even if ex…

f0987f8

…ception was raised.

test-upstream

3acadbf

Fix for pandas 1.3.

2f50ab2

More test fixes.

791242d

test-upstream

9ffd076

Fix for pandas < 1.5.

fe5ac6f

test-upstream

a6a9ae7

Fix more upstream failures.

afdf614

test-upstream

02dbbbd

j-bennet requested a review from jrbourbeau February 1, 2023 21:55

j-bennet added the upstream label Feb 1, 2023

numeric_only=False also not implemented with pandas<1.5.

88286cc

j-bennet mentioned this pull request Feb 2, 2023

numeric_only compatibility with pandas>=2.0, handling datetime and timedelta #9913

Open

Add datetimes to test data.

f75d265

jrbourbeau reviewed Feb 2, 2023

View reviewed changes

j-bennet and others added 5 commits February 2, 2023 13:33

Ignore warning decorator on tests.

04cb9b6

Ignore warning decorator on more tests, where we don't specifically c…

1d66249

…heck warning behavior.

Remove commented code.

a4a2872

Co-authored-by: James Bourbeau <[email protected]>

Review feedback.

a2b3f74

Review feedback.

0b6171e

Skip problematic test_reductions_frame_dtypes cases

a9e27d9

jrbourbeau reviewed Feb 6, 2023

View reviewed changes

jrbourbeau added 5 commits February 6, 2023 15:30

Merge branch 'main' of https://github.com/dask/dask into j-bennet/973…

211c6e8

…6-groupby-numeric-only

Updates

8c446ac

Use check_numeric_only_deprecation in mean and std

3e629d9

Don't support numeric_only for pandas<1.5

d08f275

Remove unneeded warning ignore

504c9f1

j-bennet commented Feb 9, 2023

View reviewed changes

jrbourbeau added 2 commits February 9, 2023 15:01

Factor out nuisance columns warning into own utility

e4bc650

Don't assert specific number of warnings

87d543d

jrbourbeau changed the title ~~numeric_only support for simple aggs, pandas 2.0 compatibility~~ Support numeric_only for simple groupby aggregations for pandas 2.0 compatibility Feb 10, 2023

jrbourbeau approved these changes Feb 10, 2023

View reviewed changes

jrbourbeau merged commit 052a625 into dask:main Feb 10, 2023

jrbourbeau mentioned this pull request Feb 13, 2023

Increase minimum supported versions to pandas=1.3 and numpy=1.21 #9950

Merged

j-bennet deleted the j-bennet/9736-groupby-numeric-only branch February 14, 2023 05:17

vyasr mentioned this pull request Feb 15, 2023

Stop force pulling fmt in nvbench. rapidsai/cudf#12768

Merged

3 tasks

galipremsagar mentioned this pull request Feb 15, 2023

[REVIEW] Add GroupBy.dtypes rapidsai/cudf#12783

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `numeric_only` for simple groupby aggregations for `pandas` 2.0 compatibility #9889

Support `numeric_only` for simple groupby aggregations for `pandas` 2.0 compatibility #9889

j-bennet commented Jan 27, 2023 •

edited

Loading

j-bennet commented Jan 27, 2023

j-bennet commented Jan 27, 2023 •

edited

Loading

jrbourbeau left a comment

jrbourbeau Feb 2, 2023

j-bennet Feb 2, 2023

j-bennet Feb 9, 2023

jrbourbeau Feb 9, 2023

jrbourbeau Feb 2, 2023

jrbourbeau Feb 2, 2023

j-bennet Feb 2, 2023

jrbourbeau Feb 2, 2023

j-bennet Feb 2, 2023

j-bennet commented Feb 3, 2023 •

edited

Loading

jrbourbeau commented Feb 3, 2023

jrbourbeau left a comment

j-bennet Feb 9, 2023

jrbourbeau left a comment

		assert_eq(ddf.groupby(ddf.w).y.nunique(), df.groupby(df.w).y.nunique())
		assert_eq(ddf.y.groupby(ddf.w).count(), df.y.groupby(df.w).count())

Support numeric_only for simple groupby aggregations for pandas 2.0 compatibility #9889

Support numeric_only for simple groupby aggregations for pandas 2.0 compatibility #9889

Conversation

j-bennet commented Jan 27, 2023 • edited Loading

j-bennet commented Jan 27, 2023

j-bennet commented Jan 27, 2023 • edited Loading

jrbourbeau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-bennet commented Feb 3, 2023 • edited Loading

jrbourbeau commented Feb 3, 2023

jrbourbeau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrbourbeau left a comment

Choose a reason for hiding this comment

Support `numeric_only` for simple groupby aggregations for `pandas` 2.0 compatibility #9889

Support `numeric_only` for simple groupby aggregations for `pandas` 2.0 compatibility #9889

j-bennet commented Jan 27, 2023 •

edited

Loading

j-bennet commented Jan 27, 2023 •

edited

Loading

j-bennet commented Feb 3, 2023 •

edited

Loading