Skip to content

REGR: Fix comparison broadcasting over array of Intervals #35938

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

Jump to bottom
Merged
merged 9 commits into from
Aug 31, 2020
Merged

REGR: Fix comparison broadcasting over array of Intervals #35938

merged 9 commits into from
Aug 31, 2020

Conversation

dsaxton
Copy link
Member

@dsaxton dsaxton commented Aug 28, 2020

@dsaxton dsaxton added Regression Functionality that used to work in a prior pandas version Interval Interval data type labels Aug 28, 2020
@dsaxton dsaxton added this to the 1.1.2 milestone Aug 28, 2020
@jorisvandenbossche jorisvandenbossche changed the title Don't set Interval __array_priority__ REGR: Don't set Interval __array_priority__ Aug 28, 2020
jorisvandenbossche
jorisvandenbossche reviewed Aug 28, 2020
View reviewed changes
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Can you also add a test for the array broadcasting issue you noted in #35931 (comment) ?

@dsaxton dsaxton mentioned this pull request Aug 28, 2020
Closed
@jbrockmendel
Copy link
Member

Looks like Interval.__richcmp__ returns NotImplemented in nearly all cases. I think if we handle util.is_array(other) cases directly that might solve this problem without breaking addition

@dsaxton
Copy link
Member Author

dsaxton commented Aug 30, 2020

Looks like Interval.__richcmp__ returns NotImplemented in nearly all cases. I think if we handle util.is_array(other) cases directly that might solve this problem without breaking addition

Nice, I think that's working. So far I only did a minimal copy / paste from another Cython class and stuff is passing, but am I missing some cases you can think of?

@dsaxton dsaxton changed the title REGR: Don't set Interval __array_priority__ REGR: Fix comparison broadcasting over array of Intervals Aug 30, 2020
jreback
jreback approved these changes Aug 31, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm. @jbrockmendel if any comments.

jbrockmendel
jbrockmendel reviewed Aug 31, 2020
View reviewed changes
pandas/tests/scalar/interval/test_arithmetic.py Outdated


def test_timdelta_add_timestamp_interval():
delta = np.timedelta64(0)
Copy link
Member

Choose a reason for hiding this comment

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

here or follow-up we should get the full complement of add/sub with dt64/td64/pytimedelta/Tick/...

Copy link
Member Author

Choose a reason for hiding this comment

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

Might as well parametrize over a few timedelta types here

@jbrockmendel
Copy link
Member

LGTM

@jreback jreback merged commit d7f30b4 into pandas-dev:master Aug 31, 2020
@jreback
Copy link
Contributor

jreback commented Aug 31, 2020

thanks @dsaxton very nice!

jbrockmendel pushed a commit to jbrockmendel/pandas that referenced this pull request Aug 31, 2020
@dsaxton @jbrockmendel
REGR: Fix comparison broadcasting over array of Intervals (pandas-dev…
424c011 
…#35938)
@dsaxton dsaxton deleted the interval-array-priority branch September 1, 2020 01:53
@simonjayhawkins
Copy link
Member

@meeseeksdev backport 1.1.x

@simonjayhawkins simonjayhawkins mentioned this pull request Sep 1, 2020
Closed
3 tasks
@meeseeksmachine meeseeksmachine mentioned this pull request Sep 1, 2020
Merged
meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Sep 1, 2020
@dsaxton @meeseeksmachine
Backport PR pandas-dev#35938: REGR: Fix comparison broadcasting over …
1fccd04 
…array of Intervals
simonjayhawkins pushed a commit that referenced this pull request Sep 1, 2020
@meeseeksmachine @dsaxton
Backport PR #35938: REGR: Fix comparison broadcasting over array of I…
736550d 
…ntervals (#36039)

Co-authored-by: Daniel Saxton <[email protected]>
kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@dsaxton
REGR: Fix comparison broadcasting over array of Intervals (pandas-dev…
a3d4495 
…#35938)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments

@jbrockmendel jbrockmendel jbrockmendel left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Interval Interval data type Regression Functionality that used to work in a prior pandas version
Projects
None yet
Milestone
1.1.2
Development

Successfully merging this pull request may close these issues.
5 participants
@dsaxton @jbrockmendel @jreback @simonjayhawkins @jorisvandenbossche