Skip to content

DEPR: Enforce deprecation of dropping columns when numeric_only=False in groupby / resample #49665

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

Already on GitHub? Sign in to your account

Merged

Conversation

rhshadrach
Copy link
Member

@rhshadrach rhshadrach commented Nov 12, 2022

  • closes #xxxx (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.

Prior to calling the cython code, cummin/cummax/cumsum/cumprod raise NotImplementedError on object dtypes. The Series path intercepts this and raises a TypeError instead. I've implemented the same here except for DataFrame, we can't readily tell which dtype(s) would fail, so it raises a somewhat vague "at least one dtype" message.

@rhshadrach rhshadrach added Groupby Resample resample method Deprecate Functionality to remove in pandas Nuisance Columns Identifying/Dropping nuisance columns in reductions, groupby.add, DataFrame.apply labels Nov 12, 2022
@rhshadrach rhshadrach added this to the 2.0 milestone Nov 12, 2022
arr_func, ignore_failures=numeric_only is lib.no_default
)
except NotImplementedError as err:
msg = f"{how} is not supported for at least one provided dtype"
Copy link
Member

Choose a reason for hiding this comment

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

why not just let the original exception surface? wont that include dtype information?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! That's much better.

@mroeschke mroeschke merged commit 0a58c03 into pandas-dev:main Nov 16, 2022
@mroeschke
Copy link
Member

Thanks @rhshadrach

arr_func, ignore_failures=numeric_only is lib.no_default
)
except NotImplementedError as err:
# For NotImplementedError, args[0] is the error message
Copy link
Member

Choose a reason for hiding this comment

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

is this still the case if it was raised with just raise NotImplementedError?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah - no, it's not. I'll fix in a followup. Thanks.

@rhshadrach rhshadrach deleted the enforce_no_drop_nuisance_groupby branch November 16, 2022 21:45
topper-123 pushed a commit to topper-123/pandas that referenced this pull request Nov 17, 2022
… in groupby / resample (pandas-dev#49665)

* DEPR: Enforce deprecation of dropping columns when numeric_only=False in groupby / resample

* Change to TypeError

* Better error message
MarcoGorelli pushed a commit to MarcoGorelli/pandas that referenced this pull request Nov 18, 2022
… in groupby / resample (pandas-dev#49665)

* DEPR: Enforce deprecation of dropping columns when numeric_only=False in groupby / resample

* Change to TypeError

* Better error message
mliu08 pushed a commit to mliu08/pandas that referenced this pull request Nov 27, 2022
… in groupby / resample (pandas-dev#49665)

* DEPR: Enforce deprecation of dropping columns when numeric_only=False in groupby / resample

* Change to TypeError

* Better error message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Groupby Nuisance Columns Identifying/Dropping nuisance columns in reductions, groupby.add, DataFrame.apply Resample resample method
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants