Skip to content

ENH: .equals for Extension Arrays #30652

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.

Sign up for GitHub

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
merged 19 commits into from
May 9, 2020
Merged

ENH: .equals for Extension Arrays #30652

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
36c8b88
ENH: .equals for Extension Arrays
dwhu Jan 3, 2020
786963c
ENH: Updating eq and ne methods for extension arrays.
dwhu Jan 3, 2020
6800315
Removing interval.py's __eq__ implementation due to conflict with @js…
dwhu Jan 3, 2020
a3e7b7f
ENH: Making EA eq and ne typed as Any.
dwhu Jan 3, 2020
860013f
ENH: Adding default implementation to ExtensionArray equals() and tests.
dwhu Jan 3, 2020
fc3d2c2
Merge remote-tracking branch 'upstream/master' into gh-27081
dwhu Jan 9, 2020
3da5726
Merge remote-tracking branch 'upstream/master' into gh-27081
jorisvandenbossche May 1, 2020
c5027dd
correct __eq/ne__ to be element-wise
jorisvandenbossche May 1, 2020
375664c
fix equals implementation (& instead of ==)
jorisvandenbossche May 1, 2020
b6ad2fb
base tests
jorisvandenbossche May 1, 2020
365362a
ensure to dispatch Series.equals to EA.equals
jorisvandenbossche May 1, 2020
aae2f94
Merge remote-tracking branch 'upstream/master' into gh-27081
jorisvandenbossche May 2, 2020
8d052ad
feedback: docs, whatsnew, dataframe test, strict dtype test
jorisvandenbossche May 2, 2020
9ee034e
add to reference docs
jorisvandenbossche May 2, 2020
38501e6
remove IntervalArray.__ne__
jorisvandenbossche May 2, 2020
dccec7f
type ignore following mypy issue (mypy/2783)
jorisvandenbossche May 5, 2020
0b1255f
Merge remote-tracking branch 'upstream/master' into gh-27081
jorisvandenbossche May 7, 2020
b8be858
try again without type: ignore
jorisvandenbossche May 7, 2020
4c7273f
updates
jorisvandenbossche May 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pandas/core/arrays/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def __iter__(self):
for i in range(len(self)):
yield self[i]

def __eq__(self, other: Any) -> ArrayLike:
def __eq__(self, other: Any) -> ArrayLike: # type: ignore[override] # NOQA
Copy link
Member

Choose a reason for hiding this comment

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

im not familiar with the ignore[overrride] # NOQA pattern. do we use that elsewhere?

Copy link
Member

Choose a reason for hiding this comment

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

I thought I posted the mypy issue, but apparently forgot.

So the problem here is that mypy expects a "bool" return for __eq__ since the base object is typed that way in the stubs. In python/mypy#2783, the recommended way to solve this is the above with # type: ignore[override].

But flake8 doesn't like that, hence also the #NOQA (PyCQA/pyflakes#475).

This was already done in another PR as well:

pandas/pandas/core/frame.py

Line 5062 in 7aa710a

def sort_values( # type: ignore[override] # NOQA # issue 27237

"""
Return for `self == other` (element-wise equality).
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should have some guidance here that if other is a Series then you should return NotImplemented.

Copy link
Member

Choose a reason for hiding this comment

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

could use ops.common.unpack_zerodim_and_defer

Copy link
Member

Choose a reason for hiding this comment

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

could use ops.common.unpack_zerodim_and_defer

This method is to be implemented by EA authors, so those can't use that helper (unless we expose somewhere a public version of this).

(we could of course use that for our own EAs, but this PR is not changing any existing __eq__ implementation at the moment)

Copy link
Member

Choose a reason for hiding this comment

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

I think we should have some guidance here that if other is a Series then you should return NotImplemented.

Added a comment about that

"""
Expand All @@ -347,7 +347,7 @@ def __eq__(self, other: Any) -> ArrayLike:
# underlying arrays)
raise AbstractMethodError(self)

def __ne__(self, other: Any) -> ArrayLike:
def __ne__(self, other: Any) -> ArrayLike: # type: ignore[override] # NOQA
"""
Return for `self != other` (element-wise in-equality).
"""
Expand Down