Skip to content
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

CLN: Enforce deprecation of using alias for builtin/NumPy funcs #57444

Merged
merged 17 commits into from
Feb 27, 2024

Conversation

rhshadrach
Copy link
Member

  • 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.

Ref: #53974

@rhshadrach rhshadrach added Clean Apply Apply, Aggregate, Transform, Map labels Feb 15, 2024
@rhshadrach rhshadrach added this to the 3.0 milestone Feb 15, 2024
@rhshadrach
Copy link
Member Author

A bit of a complication here - I missed one of the paths in apply where we use the builtin/NumPy -> cython table.

pandas/pandas/core/apply.py

Lines 1439 to 1442 in 54d2033

if callable(func):
f = com.get_cython_func(func)
if f and not self.args and not self.kwargs:
return obj.apply(func, by_row=False)

Sadly, it's probably one of the more common cases where users will see a difference. This is because apply will first try to operate row-by-row (unless by_row=False), and e.g. np.sum works on rows.

I'm thinking we enforce the deprecation as-is (we already were not very consistent with where we were making these replacements), and deprecate the last remaining case as part of 3.x.

cc @jorisvandenbossche @mroeschke

Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

Changes generally look good. npdev failure looks real?

I'm thinking we enforce the deprecation as-is (we already were not very consistent with where we were making these replacements), and deprecate the last remaining case as part of 3.x.

Yeah fine to do a follow up deprecation in 3.x

@jorisvandenbossche
Copy link
Member

Yes, sounds good to me too!

@rhshadrach
Copy link
Member Author

@mroeschke - green now.

@mroeschke mroeschke merged commit 47cd690 into pandas-dev:main Feb 27, 2024
47 checks passed
@mroeschke
Copy link
Member

Thanks @rhshadrach

@rhshadrach rhshadrach deleted the enf_apply_alias branch February 27, 2024 22:49
Chisholm6192 added a commit to Chisholm6192/pandas that referenced this pull request Feb 27, 2024
…as-dev#57444)

* CLN: Enforce deprecation of using alias for builtin/NumPy funcs

* GH# and whatsnew

* Fixup docs

* More tests

* Restore docstring

* Test fixes

* Test fixups

* Test fixes

* Test fixup

* Test fixes

- [ ] closes #xxxx (Replace xxxx with the GitHub issue number)
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) 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.
Chisholm6192 added a commit to Chisholm6192/pandas that referenced this pull request Feb 27, 2024
CLN: Enforce deprecation of using alias for builtin/NumPy funcs (pandas-dev#57444)
pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
…as-dev#57444)

* CLN: Enforce deprecation of using alias for builtin/NumPy funcs

* GH# and whatsnew

* Fixup docs

* More tests

* Restore docstring

* Test fixes

* Test fixups

* Test fixes

* Test fixup

* Test fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Apply Apply, Aggregate, Transform, Map Clean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants