Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

DEPR: allowing unknowns in take #52981

Merged
merged 3 commits into from
May 30, 2023
Merged

Conversation

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

@mroeschke
Copy link
Member

What's the motivation for this change?

@jbrockmendel
Copy link
Member Author

is_array_like is an antipattern #52834

@mroeschke
Copy link
Member

Seems pretty heavy to deprecate this usage just to avoid is_array_like, especially since numpy.take will accept array-likes. Why can't we just replace not is_array_like with not isinstance(..., (np.ndarray, ...)) and accept we may be calling asarray on more items?

@jbrockmendel
Copy link
Member Author

Why can't we just replace not is_array_like with not isinstance(..., (np.ndarray, ...)) and accept we may be calling asarray on more items?

That's a reasonable intuition, but np.asarray can have weird behaviors which we generally work around (but unfortunately have several slightly-different implementations of). My thought is to avoid that by just having the user do that casting.

@rhshadrach
Copy link
Member

But we'll still allow e.g. a list in DataFrame.take and others? This seems inconsistent.

@jbrockmendel
Copy link
Member Author

But we'll still allow e.g. a list in DataFrame.take and others? This seems inconsistent.

This is for the first arg in take, not the indices.

@rhshadrach
Copy link
Member

I personally wouldn't expect pandas to implement taking from a general sequence (e.g. a list); I'm okay with deprecating from that perspective.

@jbrockmendel jbrockmendel added the Deprecate Functionality to remove in pandas label May 27, 2023
@jbrockmendel
Copy link
Member Author

@mroeschke gentle ping on this and #52986

@mroeschke mroeschke added this to the 2.1 milestone May 30, 2023
@mroeschke mroeschke merged commit 6afe4a0 into pandas-dev:main May 30, 2023
@mroeschke
Copy link
Member

Thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the depr-take branch May 30, 2023 20:38
topper-123 pushed a commit to topper-123/pandas that referenced this pull request Jun 5, 2023
* DEPR: allowing unknowns in take

* GH ref
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
* DEPR: allowing unknowns in take

* GH ref
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants