Skip to content

Conversation

cmp0xff
Copy link
Contributor

@cmp0xff cmp0xff commented May 26, 2025

In #61455 (comment):

we should actually be using Hashable everywhere as since it's an actual Python type unlike label.

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

@cmp0xff cmp0xff marked this pull request as ready for review May 26, 2025 20:35
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

lgtm, I'll let @mroeschke have a look, since he was the one who suggested this.

DataFrames. If `on` is None and not merging on indexes then this defaults
to the intersection of the columns in both DataFrames.
left_on : label or list, or array-like
left_on : Hashable or a sequence of the previous, or array-like
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if a sequence of the previous already means array-like. No big deal, but I guess it could be removed.

Copy link
Contributor Author

@cmp0xff cmp0xff May 27, 2025

Choose a reason for hiding this comment

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

My experience was that numpy.typing.NDArray is somehow not a collections.abc.Sequence, and numpy.typing.ArrayLike is even more complicated than an NDArray. That was why I left them as separate things.

@mroeschke mroeschke added this to the 3.0 milestone May 27, 2025
@mroeschke mroeschke merged commit 1ccdfe6 into pandas-dev:main May 27, 2025
48 checks passed
@mroeschke
Copy link
Member

Thanks @cmp0xff

@cmp0xff cmp0xff deleted the feature/cmp0xff/use-hashable branch May 27, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants