WARN introduce FutureWarning for value_counts behaviour change #49640

MarcoGorelli · 2022-11-11T10:46:17Z

xref API: various .value_counts() result in different names / indices #49497 (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

As discussed in the dev call:

the new behaviour would be more desirable than the current one;
warning about it if users don't pass name= would be very noisy
if we just warn for 1.5.2, then there's at least some warning and it won't be there for too long as 2.0.0 should be out in a few months' time

Note that no warning is introduced for

df.groupby(col, as_index=False).value_counts()

as that's the one that the other value_counts will conform to

rhshadrach · 2022-11-11T13:10:02Z

pandas/core/groupby/generic.py

@@ -604,7 +604,19 @@ def value_counts(
        ascending: bool = False,
        bins=None,
        dropna: bool = True,
+        *,
+        name: Hashable | None = None,


This is adding name to the API, which would presumably be deprecated in 2.0. So users would be adding name to silence the warning, then removing it to silence the new deprecation warning.

My understanding of the proposal was to just add a warning - no arguments - that the user will see on every use. While that is very noisy (maybe we could do a DeprecationWarning instead?), I think it's better than having users change code only to change it back.

cc @jreback

thanks @rhshadrach for taking a look - yeah if it's OK to emit a warning each time then that'd be better, I'm also not keen on adding something to the API and then immediately deprecating it. I'll do that

jreback

this going to be extremely noisy as implemented

jreback · 2022-11-12T19:50:30Z

doc/source/getting_started/comparison/includes/filtering.rst

@@ -9,6 +9,7 @@ The above statement is simply passing a ``Series`` of ``True``/``False`` objects
 returning all rows with ``True``.

 .. ipython:: python
+    :okwarning:


fix the docs rather that ignoring the warnings

jreback · 2022-11-12T19:51:07Z

doc/source/whatsnew/v0.11.0.rst

@@ -289,6 +289,7 @@ Furthermore ``datetime64[ns]`` columns are created by default, when passed datet
 (:issue:`2809`, :issue:`2810`)

 .. ipython:: python
+   :okwarning:


older release notes ok to ignore (or convert to code blocks )

jreback · 2022-11-12T19:51:52Z

doc/source/whatsnew/v1.5.2.rst

@@ -33,7 +33,7 @@ Bug fixes

 Other
 ~~~~~
-
+- Introduced ``FutureWarning`` notifying about behaviour change in :meth:`DataFrame.value_counts`, :meth:`Series.value_counts`, :meth:`DataFrameGroupBy.value_counts`, :meth:`SeriesGroupBy.value_counts` - the resulting series will by default now be named ``'counts'`` (or ``'proportion'`` if ``normalize=True``), and the index (if present) will be taken from the original object's name (:issue:`49497`)


this warrants a full on section and before / after on how to fix

jreback · 2022-11-12T19:52:27Z

pandas/core/describe.py

+    with warnings.catch_warnings():
+        msg = "In pandas 2.0.0, the name of the resulting Series"
+        warnings.filterwarnings("ignore", msg, FutureWarning)
+        objcounts = data.value_counts()


can we fix it here? the fact that u have to catch is a problem

jreback · 2022-11-12T19:53:13Z

pandas/core/frame.py

+        warnings.warn(
+            "In pandas 2.0.0, the name of the resulting Series will be "
+            "'count' (or 'proportion' if `normalize=True`).",
+            FutureWarning,


this is going to be extremely noisy

is there a way to do so that it doesn't show the warnjng

Responding to #49640 (comment) as well.

I thought @jreback expressed the idea of having the warning (with no way to silence) on the call; it seems like I misunderstood. My preferred route would be no warning; another option is to just have a DeprecationWarning - anyone running an interactive session won't see the warning, but those testing via pytest would. While I dislike the idea of adding name, I'm not -1 on it.

From the linked issue on adding name, I'm taking some liberties here but it seems @jorisvandenbossche is +1, @jbrockmendel is -0.5, @Dr-Irv is +1, and @MarcoGorelli was -1.

I presume there will be a 1.5.3 release before 2.0.0? If so, perhaps let's put this off until then, don't want to rush a decision

I wouldn't be keen on adding name, but I'd still prefer that to not making the change

if we did a DeprecationWarnjng though then this just is really a breaking change and we should just do it

so either am ok with either

just change it in 2.0

engineer a nice way to warn in 1.5.x but cannot be so noisy it's everywhere

MarcoGorelli · 2022-11-12T20:00:42Z

there's conflicting reviews now 😄 #49640 (comment)

Having been through adjusting all the warnings in this PR, I am concerned about noise: df.groupby(col).apply(lambda df: df.value_counts()) in particular would emit multiple warnings

@rhshadrach would you be OK with adding a name parameter to ease the deprecation cycle? The only other alternative (other than not doing this) I can think of would be to make the change without warning

MarcoGorelli · 2022-11-14T15:08:26Z

closing for now as it's not the right solution, will a comment to the issue with further thoughts

wip

25bef51

MarcoGorelli force-pushed the value-counts-names branch from a2d9096 to 25bef51 Compare November 11, 2022 11:34

rhshadrach reviewed Nov 11, 2022

View reviewed changes

MarcoGorelli added 8 commits November 11, 2022 15:46

warn regardless

784cdbf

Merge remote-tracking branch 'upstream/main' into value-counts-names

7d4be12

clean up

1c0d22b

remove dead tests

098596a

:okwarning: in docs

4160b9b

comment about catch_warnings

29e3386

comment about catch_warnings

c02628e

catch more warnings

b672854

MarcoGorelli marked this pull request as ready for review November 11, 2022 19:04

MarcoGorelli added this to the 1.5.2 milestone Nov 11, 2022

MarcoGorelli added 2 commits November 11, 2022 19:32

filter warning in test_string

4b3d59d

catch test_arrow warning

66e530f

MarcoGorelli mentioned this pull request Nov 11, 2022

RLS: pandas 1.5.2 #49194

Closed

MarcoGorelli added 9 commits November 12, 2022 06:10

filterwarnings

401d595

Merge remote-tracking branch 'upstream/main' into value-counts-names

3c67b7d

catch warnings

4c76219

clean up

1a11d65

use warnings.filterwarnings in tests for now

8cafda4

pyarrow < 7.0.0 tests adjustments

b3f3984

catch warnings in other functions

210e624

clean up

c9acbf2

fix okwarning indent

0c7f2d9

jreback requested changes Nov 12, 2022

View reviewed changes

MarcoGorelli closed this Nov 14, 2022

MarcoGorelli mentioned this pull request Nov 14, 2022

API: various .value_counts() result in different names / indices #49497

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WARN introduce FutureWarning for value_counts behaviour change #49640

WARN introduce FutureWarning for value_counts behaviour change #49640

MarcoGorelli commented Nov 11, 2022

rhshadrach Nov 11, 2022 •

edited

Loading

MarcoGorelli Nov 11, 2022

jreback left a comment

jreback Nov 12, 2022

jreback Nov 12, 2022

jreback Nov 12, 2022

jreback Nov 12, 2022

jreback Nov 12, 2022

rhshadrach Nov 12, 2022

MarcoGorelli Nov 13, 2022

jreback Nov 13, 2022

MarcoGorelli commented Nov 12, 2022 •

edited

Loading

MarcoGorelli commented Nov 14, 2022

WARN introduce FutureWarning for value_counts behaviour change #49640

WARN introduce FutureWarning for value_counts behaviour change #49640

Conversation

MarcoGorelli commented Nov 11, 2022

rhshadrach Nov 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback 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

MarcoGorelli commented Nov 12, 2022 • edited Loading

MarcoGorelli commented Nov 14, 2022

rhshadrach Nov 11, 2022 •

edited

Loading

MarcoGorelli commented Nov 12, 2022 •

edited

Loading